From 3c875c36ab9a5161692345889e7557411bfab7bf Mon Sep 17 00:00:00 2001 From: Richard Watts Date: Wed, 29 Mar 2023 20:18:07 +0100 Subject: [PATCH 1/2] (fix) ZIL-5144: Don't send Scilla contract creation to ARCHIVAL_SEND_SHARD unless you are actually a lookup or archival node. --- src/libServer/LookupServer.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/libServer/LookupServer.cpp b/src/libServer/LookupServer.cpp index 6be2229be1..3c98141e0b 100644 --- a/src/libServer/LookupServer.cpp +++ b/src/libServer/LookupServer.cpp @@ -2391,7 +2391,9 @@ std::pair LookupServer::CheckContractTxnShards( if (!tx.IsEth() && scType == Transaction::CONTRACT_CREATION) { // Scilla smart CONTRACT_CREATION call should be executed in shard rather // than DS. - mapIndex = SEND_TYPE::ARCHIVAL_SEND_SHARD; + if (ARCHIVAL_LOOKUP) { + mapIndex = SEND_TYPE::ARCHIVAL_SEND_SHARD; + } resultStr = "Contract Creation txn, sent to shard"; } else { // CONTRACT_CALL - scilla and EVM , CONTRACT_CREATION - EVM From 036a706793fbe10d0e809f7008187f271f11fd0e Mon Sep 17 00:00:00 2001 From: Valeriy Zamaraiev Date: Thu, 30 Mar 2023 14:56:32 +0200 Subject: [PATCH 2/2] refactor --- src/libServer/EthRpcMethods.cpp | 69 +++------------------------------ src/libServer/EthRpcMethods.h | 4 +- src/libServer/LookupServer.cpp | 67 +++++--------------------------- src/libServer/LookupServer.h | 7 ++-- 4 files changed, 22 insertions(+), 125 deletions(-) diff --git a/src/libServer/EthRpcMethods.cpp b/src/libServer/EthRpcMethods.cpp index 5db0f7c252..e7109d8e3e 100644 --- a/src/libServer/EthRpcMethods.cpp +++ b/src/libServer/EthRpcMethods.cpp @@ -463,7 +463,6 @@ std::string EthRpcMethods::CreateTransactionEth( const unsigned int shard = Transaction::GetShardIndex(fromAddr, num_shards); unsigned int mapIndex = shard; - bool priority = false; switch (Transaction::GetTransactionType(tx)) { case Transaction::ContractType::NON_CONTRACT: if (ARCHIVAL_LOOKUP) { @@ -473,7 +472,7 @@ std::string EthRpcMethods::CreateTransactionEth( // A simple transfer to an account that is a contract // is processed like a CONTRACT_CALL. auto check = - CheckContractTxnShards(priority, shard, tx, num_shards, + CheckContractTxnShards(tx, num_shards, toAccountExist, toAccountIsContract); mapIndex = check.second; } @@ -481,7 +480,7 @@ std::string EthRpcMethods::CreateTransactionEth( case Transaction::ContractType::CONTRACT_CREATION: case Transaction::ContractType::CONTRACT_CALL: { auto check = - CheckContractTxnShards(priority, shard, tx, num_shards, + CheckContractTxnShards(tx, num_shards, toAccountExist, toAccountIsContract); mapIndex = check.second; } break; @@ -517,66 +516,10 @@ std::string EthRpcMethods::CreateTransactionEth( } std::pair EthRpcMethods::CheckContractTxnShards( - bool priority, unsigned int shard, const Transaction &tx, - unsigned int num_shards, bool toAccountExist, bool toAccountIsContract) { - INC_CALLS(GetInvocationsCounter()); - - unsigned int mapIndex = shard; - std::string resultStr; - - if (!ENABLE_SC) { - throw JsonRpcException(ServerBase::RPC_MISC_ERROR, - "Smart contract is disabled"); - } - - if (!toAccountExist) { - throw JsonRpcException(ServerBase::RPC_INVALID_ADDRESS_OR_KEY, - "Target account does not exist"); - } else if (Transaction::GetTransactionType(tx) == - Transaction::CONTRACT_CALL && - !toAccountIsContract) { - throw JsonRpcException(ServerBase::RPC_INVALID_ADDRESS_OR_KEY, - "Non - contract address called"); - } - - Address affectedAddress = - (Transaction::GetTransactionType(tx) == Transaction::CONTRACT_CREATION) - ? Account::GetAddressForContract(tx.GetSenderAddr(), tx.GetNonce(), - tx.GetVersionIdentifier()) - : tx.GetToAddr(); - - unsigned int to_shard = - Transaction::GetShardIndex(affectedAddress, num_shards); - // Use m_sendSCCallsToDS as initial setting - bool sendToDs = priority || m_sharedMediator.m_lookup->m_sendSCCallsToDS; - if ((to_shard == shard) && !sendToDs) { - if (tx.GetGasLimitZil() > SHARD_MICROBLOCK_GAS_LIMIT) { - throw JsonRpcException(ServerBase::RPC_INVALID_PARAMETER, - "txn gas limit exceeding shard maximum limit"); - } - if (ARCHIVAL_LOOKUP) { - mapIndex = SEND_TYPE::ARCHIVAL_SEND_SHARD; - } - resultStr = - "Contract Creation/Call Txn, Shards Match of the sender " - "and receiver"; - } else { - if (tx.GetGasLimitZil() > DS_MICROBLOCK_GAS_LIMIT) { - throw JsonRpcException( - ServerBase::RPC_INVALID_PARAMETER, - (boost::format( - "txn gas limit exceeding ds maximum limit! Tx: %i DS: %i") % - tx.GetGasLimitZil() % DS_MICROBLOCK_GAS_LIMIT) - .str()); - } - if (ARCHIVAL_LOOKUP) { - mapIndex = SEND_TYPE::ARCHIVAL_SEND_DS; - } else { - mapIndex = num_shards; - } - resultStr = "Contract Creation/Call Txn, Sent To Ds"; - } - return make_pair(resultStr, mapIndex); + const Transaction &tx, unsigned int num_shards, bool toAccountExist, + bool toAccountIsContract) { + return m_lookupServer->CheckContractTxnShards(tx, num_shards, toAccountExist, + toAccountIsContract); } Json::Value EthRpcMethods::GetBalanceAndNonce(const string &address) { diff --git a/src/libServer/EthRpcMethods.h b/src/libServer/EthRpcMethods.h index 3bf6ccf801..544bafabfa 100644 --- a/src/libServer/EthRpcMethods.h +++ b/src/libServer/EthRpcMethods.h @@ -39,8 +39,8 @@ class EthRpcMethods { "Calls to ethereum API", "Calls"}; std::pair CheckContractTxnShards( - bool priority, unsigned int shard, const Transaction& tx, - unsigned int num_shards, bool toAccountExist, bool toAccountIsContract); + const Transaction& tx, unsigned int num_shards, bool toAccountExist, + bool toAccountIsContract); CreateTransactionTargetFunc m_createTransactionTarget = [this](const Transaction& tx, uint32_t shardId) -> bool { diff --git a/src/libServer/LookupServer.cpp b/src/libServer/LookupServer.cpp index 3c98141e0b..8035b7bdda 100644 --- a/src/libServer/LookupServer.cpp +++ b/src/libServer/LookupServer.cpp @@ -610,10 +610,6 @@ Json::Value LookupServer::CreateTransaction( const unsigned int shard = Transaction::GetShardIndex(fromAddr, num_shards); unsigned int mapIndex = shard; - bool priority = false; - if (_json.isMember("priority")) { - priority = _json["priority"].asBool(); - } switch (Transaction::GetTransactionType(tx)) { case Transaction::ContractType::NON_CONTRACT: if (ARCHIVAL_LOOKUP) { @@ -633,9 +629,8 @@ Json::Value LookupServer::CreateTransaction( // We use the same logic for CONTRACT_CREATION and CONTRACT_CALL. // TODO(valeryz): once we stop using Zilliqa APIs for EVM, revert // to the old behavior where CONTRACT_CREATION can be sharded. - auto check = - CheckContractTxnShards(priority, shard, tx, num_shards, - toAccountExist, toAccountIsContract); + auto check = CheckContractTxnShards(tx, num_shards, toAccountExist, + toAccountIsContract); ret["Info"] = check.first; ret["ContractAddress"] = Account::GetAddressForContract(fromAddr, tx.GetNonce() - 1, @@ -644,9 +639,8 @@ Json::Value LookupServer::CreateTransaction( mapIndex = check.second; } break; case Transaction::ContractType::CONTRACT_CALL: { - auto check = - CheckContractTxnShards(priority, shard, tx, num_shards, - toAccountExist, toAccountIsContract); + auto check = CheckContractTxnShards(tx, num_shards, toAccountExist, + toAccountIsContract); ret["Info"] = check.first; mapIndex = check.second; } break; @@ -2359,15 +2353,12 @@ Json::Value LookupServer::GetStateProof(const string& address, } std::pair LookupServer::CheckContractTxnShards( - bool priority, unsigned int shard, const Transaction& tx, + const Transaction& tx, unsigned int num_shards, bool toAccountExist, bool toAccountIsContract) { TRACE(zil::trace::FilterClass::DEMO); INC_CALLS(GetCallsCounter()); - unsigned int mapIndex = shard; - std::string resultStr; - if (!ENABLE_SC) { throw JsonRpcException(ServerBase::RPC_MISC_ERROR, "Smart contract is disabled"); @@ -2384,49 +2375,11 @@ std::pair LookupServer::CheckContractTxnShards( "Non - contract address called"); } - Transaction::ContractType scType = Transaction::GetTransactionType(tx); - - // Use m_sendSCCallsToDS as initial setting - bool sendToDs = priority || m_sharedMediator.m_lookup->m_sendSCCallsToDS; - if (!tx.IsEth() && scType == Transaction::CONTRACT_CREATION) { - // Scilla smart CONTRACT_CREATION call should be executed in shard rather - // than DS. - if (ARCHIVAL_LOOKUP) { - mapIndex = SEND_TYPE::ARCHIVAL_SEND_SHARD; - } - resultStr = "Contract Creation txn, sent to shard"; + unsigned int mapIndex; + if (ARCHIVAL_LOOKUP) { + mapIndex = SEND_TYPE::ARCHIVAL_SEND_DS; } else { - // CONTRACT_CALL - scilla and EVM , CONTRACT_CREATION - EVM - Address affectedAddress = tx.GetToAddr(); - unsigned int to_shard = - Transaction::GetShardIndex(affectedAddress, num_shards); - if ((to_shard == shard) && !sendToDs) { - if (tx.GetGasLimitZil() > SHARD_MICROBLOCK_GAS_LIMIT) { - throw JsonRpcException(ServerBase::RPC_INVALID_PARAMETER, - "txn gas limit exceeding shard maximum limit"); - } - if (ARCHIVAL_LOOKUP) { - mapIndex = SEND_TYPE::ARCHIVAL_SEND_SHARD; - } - resultStr = - "Contract Creation/Call Txn, Shards Match of the sender " - "and receiver"; - } else { - if (tx.GetGasLimitZil() > DS_MICROBLOCK_GAS_LIMIT) { - throw JsonRpcException( - ServerBase::RPC_INVALID_PARAMETER, - (boost::format( - "txn gas limit exceeding ds maximum limit! Tx: %i DS: %i") % - tx.GetGasLimitZil() % DS_MICROBLOCK_GAS_LIMIT) - .str()); - } - if (ARCHIVAL_LOOKUP) { - mapIndex = SEND_TYPE::ARCHIVAL_SEND_DS; - } else { - mapIndex = num_shards; - } - resultStr = "Contract Creation/Call Txn, Sent To Ds"; - } + mapIndex = num_shards; } - return make_pair(resultStr, mapIndex); + return make_pair("Contract Creation/Call Txn, Sent To Ds", mapIndex); } diff --git a/src/libServer/LookupServer.h b/src/libServer/LookupServer.h index 1b9c876784..52fd0c3902 100644 --- a/src/libServer/LookupServer.h +++ b/src/libServer/LookupServer.h @@ -59,9 +59,6 @@ class LookupServer : public Server, Json::Value GetTransactionsForTxBlock(const std::string& txBlockNum, const std::string& pageNumber); - std::pair CheckContractTxnShards( - bool priority, unsigned int shard, const Transaction& tx, - unsigned int num_shards, bool toAccountExist, bool toAccountIsContract); mp::cpp_dec_float_50 CalculateTotalSupply(); public: @@ -362,6 +359,10 @@ class LookupServer : public Server, bool StartCollectorThread(); std::string GetNodeState(); + std::pair CheckContractTxnShards( + const Transaction& tx, unsigned int num_shards, bool toAccountExist, + bool toAccountIsContract); + static void AddToRecentTransactions(const dev::h256& txhash); // gets the number of transaction starting from block blockNum to most recent