From b5830201c69fa891877f5ba9c68f0264a3ed56c3 Mon Sep 17 00:00:00 2001 From: iulianpascalau Date: Fri, 26 Jul 2019 14:57:22 +0300 Subject: [PATCH 1/5] fixed bug that incorrectly updated the sender balance when executing an intra shard tx that has an output to that same sender account. added integration test --- .../singleShard/block/agarioV2.hex | 1 + .../block/executingMiniblocksSc_test.go | 342 ++++++++++++++++++ process/smartContract/process.go | 13 + process/smartContract/process_test.go | 30 +- 4 files changed, 382 insertions(+), 4 deletions(-) create mode 100644 integrationTests/singleShard/block/agarioV2.hex create mode 100644 integrationTests/singleShard/block/executingMiniblocksSc_test.go diff --git a/integrationTests/singleShard/block/agarioV2.hex b/integrationTests/singleShard/block/agarioV2.hex new file mode 100644 index 00000000000..7bbd848e053 --- /dev/null +++ b/integrationTests/singleShard/block/agarioV2.hex @@ -0,0 +1 @@ +000004656305690010706C6179657242616C616E63654B657969000F706C617965725374617475734B657969000E67616D65506C61796572734B657969000A67616D654265744B657969000962616C616E63654F66690005746F705570690008776974686472617769001A7472616E736665724261636B546F506C6179657257616C6C657469000B7769746864726177416C6C69000F616464506C61796572546F47616D656900086A6F696E47616D6569001A616464506C61796572546F47616D6553746174654368616E676569000C72657761726457696E6E6572690015726577617264416E6453656E64546F57616C6C6574690007656E6447616D656900076465706F73697467000000003300618001550020670001000161010161820100021B0823170064F6000104670002000161020161820100021B0823170064F600010467000300016103016120021B0823170064F600010467000400016104016120021B0823170064F6000104680005000161A0FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF0111040265000002F80001000100010003540064F6000104660000FE680006000033003401F80001000100010002540043010463550043F6000068000700013301F80008000200000001F600006800080002F80001000100010002540043100464650000040304635500435705F20010000000000080A665000006F60000660000FE68000900003300F80001000100010001540022618003140C44650000046180035500235705F20010000000000100A665000106660000F60000660001FE68000A000333036180045400850F146665000006F8000C000300000820F60000660000FE68000B000133013402F8000600000000F8000C000300000820F60000660000FE67000C000361A0FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF03110C2465000004F800020001000100255400A66101035500A3F800010001000100275400E8100909650000090309085500E8F8000300010001000A54014B61820100031B0D6B17056BF8000300010001000A55014BF8000400010001000C54018D0109AD55018DF60000660000FE68000D000333036180045400850F14666500000661A0FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF0411102665000006F800040001000100075400E8100906650000060309085500E8F8000100010001002954012A01094A55012AF60000660000FE68000E0003F8000D000300000820F80008000200000041F60000660000FE68000F000133016180025400430F0C2465000304F800040001000100055400A6F800030001000100075400E86180021409046500030461010261820100091B244A61010203094A618002618201000903244B66000016290C61800214098D6500010DF8000200010001018E6180025501C21B2D0860018F610102650000026600016180025500E26180021408D0650002106180021409E465000304F8000E0003000019E0660002F60000660003FE \ No newline at end of file diff --git a/integrationTests/singleShard/block/executingMiniblocksSc_test.go b/integrationTests/singleShard/block/executingMiniblocksSc_test.go new file mode 100644 index 00000000000..faf92133631 --- /dev/null +++ b/integrationTests/singleShard/block/executingMiniblocksSc_test.go @@ -0,0 +1,342 @@ +package block + +import ( + "context" + "encoding/hex" + "fmt" + "io/ioutil" + "math/big" + "testing" + "time" + + "github.com/ElrondNetwork/elrond-go/core/logger" + "github.com/ElrondNetwork/elrond-go/data/state" + "github.com/ElrondNetwork/elrond-go/data/transaction" + "github.com/ElrondNetwork/elrond-go/integrationTests" + "github.com/ElrondNetwork/elrond-go/sharding" + "github.com/stretchr/testify/assert" +) + +var agarioFile = "agarioV2.hex" +var stepDelay = time.Second + +func TestShouldProcessBlocksWithScTxsJoinAndRewardTheOwner(t *testing.T) { + if testing.Short() { + t.Skip("this is not a short test") + } + + log := logger.DefaultLogger() + log.SetLevel(logger.LogDebug) + + scCode, err := ioutil.ReadFile(agarioFile) + assert.Nil(t, err) + + maxShards := uint32(1) + numOfNodes := 1 + advertiser := integrationTests.CreateMessengerWithKadDht(context.Background(), "") + _ = advertiser.Bootstrap() + advertiserAddr := integrationTests.GetConnectableAddress(advertiser) + + nodes := make([]*integrationTests.TestProcessorNode, numOfNodes) + for i := 0; i < numOfNodes; i++ { + nodes[i] = integrationTests.NewTestProcessorNode(maxShards, 0, 0, advertiserAddr) + } + + idxProposer := 0 + hardCodedSk, _ := hex.DecodeString("5561d28b0d89fa425bbbf9e49a018b5d1e4a462c03d2efce60faf9ddece2af06") + hardCodedScResultingAddress, _ := hex.DecodeString("000000000000000000005fed9c659422cd8429ce92f8973bba2a9fb51e0eb3a1") + nodes[idxProposer].LoadTxSignSkBytes(hardCodedSk) + + defer func() { + _ = advertiser.Close() + for _, n := range nodes { + _ = n.Messenger.Close() + } + }() + + for _, n := range nodes { + _ = n.Messenger.Bootstrap() + } + + fmt.Println("Delaying for nodes p2p bootstrap...") + time.Sleep(stepDelay) + + round := uint32(0) + round = incrementAndPrintRound(round) + + initialVal := big.NewInt(10000000) + topUpValue := big.NewInt(500) + withdrawValue := big.NewInt(10) + stepMintAllNodes(nodes, initialVal) + + stepDeployScTx(nodes, idxProposer, string(scCode)) + stepProposeBlock(nodes, idxProposer, round) + stepSyncBlock(t, nodes, idxProposer, round) + round = incrementAndPrintRound(round) + + stepNodeDoesJoinGame(nodes, idxProposer, topUpValue, hardCodedScResultingAddress) + stepProposeBlock(nodes, idxProposer, round) + stepSyncBlock(t, nodes, idxProposer, round) + round = incrementAndPrintRound(round) + + stepCheckJoinGameIsDoneCorrectly( + t, + nodes, + idxProposer, + idxProposer, + initialVal, + topUpValue, + hardCodedScResultingAddress, + ) + + stepNodeCallsRewardAndSend(nodes, idxProposer, idxProposer, withdrawValue, hardCodedScResultingAddress) + stepProposeBlock(nodes, idxProposer, round) + stepSyncBlock(t, nodes, idxProposer, round) + round = incrementAndPrintRound(round) + + stepProposeBlock(nodes, idxProposer, round) + stepSyncBlock(t, nodes, idxProposer, round) + round = incrementAndPrintRound(round) + + stepCheckRewardIsDoneCorrectly( + t, + nodes, + idxProposer, + idxProposer, + initialVal, + topUpValue, + withdrawValue, + hardCodedScResultingAddress, + ) + + stepCheckRootHashes(t, nodes, []int{0}) + + time.Sleep(1 * time.Second) +} + +func incrementAndPrintRound(round uint32) uint32 { + round++ + fmt.Printf("#################################### ROUND %d BEGINS ####################################\n\n", round) + + return round +} + +func stepMintAllNodes(nodes []*integrationTests.TestProcessorNode, value *big.Int) { + for _, n := range nodes { + if n.ShardCoordinator.SelfId() == sharding.MetachainShardId { + continue + } + + for _, n2 := range nodes { + integrationTests.MintAddress(n.AccntState, n2.PkTxSignBytes, value) + } + } +} + +func stepDeployScTx(nodes []*integrationTests.TestProcessorNode, senderIdx int, scCode string) { + fmt.Println("Deploying SC...") + txDeploy := createTxDeploy(nodes[senderIdx], scCode) + nodes[senderIdx].SendTransaction(txDeploy) + fmt.Println("Delaying for disseminating the deploy tx...") + time.Sleep(stepDelay) + + fmt.Println(integrationTests.MakeDisplayTable(nodes)) +} + +func stepProposeBlock(nodes []*integrationTests.TestProcessorNode, idxProposer int, round uint32) { + fmt.Println("Proposing block...") + for idx, n := range nodes { + if idx != idxProposer { + continue + } + + body, header := n.ProposeBlockOnlyWithSelf(round) + n.BroadcastAndCommit(body, header) + } + + fmt.Println("Delaying for disseminating headers and miniblocks...") + time.Sleep(stepDelay) + fmt.Println(integrationTests.MakeDisplayTable(nodes)) +} + +func stepSyncBlock(t *testing.T, nodes []*integrationTests.TestProcessorNode, idxProposer int, round uint32) { + fmt.Println("All other shard nodes sync the proposed block...") + for idx, n := range nodes { + if idx == idxProposer { + continue + } + + err := n.SyncNode(uint64(round)) + if err != nil { + assert.Fail(t, err.Error()) + return + } + } + + time.Sleep(stepDelay) + fmt.Println(integrationTests.MakeDisplayTable(nodes)) +} + +func stepNodeDoesJoinGame( + nodes []*integrationTests.TestProcessorNode, + idxNode int, + joinGameVal *big.Int, + scAddress []byte) { + + fmt.Println("Calling SC.joinGame...") + txDeploy := createTxJoinGame(nodes[idxNode], joinGameVal, scAddress) + nodes[idxNode].SendTransaction(txDeploy) + fmt.Println("Delaying for disseminating SC call tx...") + time.Sleep(stepDelay) + + fmt.Println(integrationTests.MakeDisplayTable(nodes)) +} + +func stepNodeCallsRewardAndSend( + nodes []*integrationTests.TestProcessorNode, + idxNodeOwner int, + idxNodeUser int, + prize *big.Int, + scAddress []byte) { + + fmt.Println("Calling SC.rewardAndSendToWallet...") + txDeploy := createTxRewardAndSendToWallet(nodes[idxNodeOwner], nodes[idxNodeUser], prize, scAddress) + nodes[idxNodeOwner].SendTransaction(txDeploy) + fmt.Println("Delaying for disseminating SC call tx...") + time.Sleep(time.Second * 1) + + fmt.Println(integrationTests.MakeDisplayTable(nodes)) +} + +func createTxDeploy(tn *integrationTests.TestProcessorNode, scCode string) *transaction.Transaction { + tx := &transaction.Transaction{ + Nonce: 0, + Value: big.NewInt(0), + RcvAddr: make([]byte, 32), + SndAddr: tn.PkTxSignBytes, + Data: scCode, + GasPrice: 0, + GasLimit: 100000, + } + txBuff, _ := integrationTests.TestMarshalizer.Marshal(tx) + tx.Signature, _ = tn.SingleSigner.Sign(tn.SkTxSign, txBuff) + + return tx +} + +func createTxJoinGame(tn *integrationTests.TestProcessorNode, joinGameVal *big.Int, scAddress []byte) *transaction.Transaction { + tx := &transaction.Transaction{ + Nonce: 0, + Value: joinGameVal, + RcvAddr: scAddress, + SndAddr: tn.PkTxSignBytes, + Data: fmt.Sprintf("joinGame@aaaa"), + GasPrice: 0, + GasLimit: 100000, + } + txBuff, _ := integrationTests.TestMarshalizer.Marshal(tx) + tx.Signature, _ = tn.SingleSigner.Sign(tn.SkTxSign, txBuff) + + fmt.Printf("Join %s\n", hex.EncodeToString(tn.PkTxSignBytes)) + + return tx +} + +func createTxRewardAndSendToWallet(tnOwner *integrationTests.TestProcessorNode, tnUser *integrationTests.TestProcessorNode, prizeVal *big.Int, scAddress []byte) *transaction.Transaction { + tx := &transaction.Transaction{ + Nonce: 0, + Value: big.NewInt(0), + RcvAddr: scAddress, + SndAddr: tnOwner.PkTxSignBytes, + Data: fmt.Sprintf("rewardAndSendToWallet@aaaa@%s@%X", hex.EncodeToString(tnUser.PkTxSignBytes), prizeVal), + GasPrice: 0, + GasLimit: 100000, + } + txBuff, _ := integrationTests.TestMarshalizer.Marshal(tx) + tx.Signature, _ = tnOwner.SingleSigner.Sign(tnOwner.SkTxSign, txBuff) + + fmt.Printf("Reward %s\n", hex.EncodeToString(tnUser.PkTxSignBytes)) + + return tx +} + +func stepCheckJoinGameIsDoneCorrectly( + t *testing.T, + nodes []*integrationTests.TestProcessorNode, + idxNodeScExists int, + idxNodeCallerExists int, + initialVal *big.Int, + topUpVal *big.Int, + scAddressBytes []byte, +) { + + nodeWithSc := nodes[idxNodeScExists] + nodeWithCaller := nodes[idxNodeCallerExists] + + fmt.Println("Checking SC account received topUp val...") + accnt, _ := nodeWithSc.AccntState.GetExistingAccount(integrationTests.CreateAddresFromAddrBytes(scAddressBytes)) + assert.NotNil(t, accnt) + assert.Equal(t, topUpVal, accnt.(*state.Account).Balance) + + fmt.Println("Checking sender has initial-topUp val...") + expectedVal := big.NewInt(0).Set(initialVal) + expectedVal.Sub(expectedVal, topUpVal) + fmt.Printf("Checking %s\n", hex.EncodeToString(nodeWithCaller.PkTxSignBytes)) + accnt, _ = nodeWithCaller.AccntState.GetExistingAccount(integrationTests.CreateAddresFromAddrBytes(nodeWithCaller.PkTxSignBytes)) + assert.NotNil(t, accnt) + assert.Equal(t, expectedVal, accnt.(*state.Account).Balance) +} + +func stepCheckRewardIsDoneCorrectly( + t *testing.T, + nodes []*integrationTests.TestProcessorNode, + idxNodeScExists int, + idxNodeCallerExists int, + initialVal *big.Int, + topUpVal *big.Int, + withdraw *big.Int, + scAddressBytes []byte, +) { + + nodeWithSc := nodes[idxNodeScExists] + nodeWithCaller := nodes[idxNodeCallerExists] + + fmt.Println("Checking SC account has topUp-withdraw val...") + accnt, _ := nodeWithSc.AccntState.GetExistingAccount(integrationTests.CreateAddresFromAddrBytes(scAddressBytes)) + assert.NotNil(t, accnt) + expectedSC := big.NewInt(0).Set(topUpVal) + expectedSC.Sub(expectedSC, withdraw) + assert.Equal(t, expectedSC, accnt.(*state.Account).Balance) + + fmt.Println("Checking sender has initial-topUp+withdraw val...") + expectedSender := big.NewInt(0).Set(initialVal) + expectedSender.Sub(expectedSender, topUpVal) + expectedSender.Add(expectedSender, withdraw) + fmt.Printf("Checking %s\n", hex.EncodeToString(nodeWithCaller.PkTxSignBytes)) + accnt, _ = nodeWithCaller.AccntState.GetExistingAccount(integrationTests.CreateAddresFromAddrBytes(nodeWithCaller.PkTxSignBytes)) + assert.NotNil(t, accnt) + assert.Equal(t, expectedSender, accnt.(*state.Account).Balance) +} + +func stepCheckRootHashes(t *testing.T, nodes []*integrationTests.TestProcessorNode, idxProposers []int) { + for _, idx := range idxProposers { + checkRootHashInShard(t, nodes, idx) + } +} + +func checkRootHashInShard(t *testing.T, nodes []*integrationTests.TestProcessorNode, idxProposer int) { + proposerNode := nodes[idxProposer] + proposerRootHash, _ := proposerNode.AccntState.RootHash() + + for i := 0; i < len(nodes); i++ { + node := nodes[i] + + if node.ShardCoordinator.SelfId() != proposerNode.ShardCoordinator.SelfId() { + continue + } + + fmt.Printf("Testing roothash for node index %d, shard ID %d...\n", i, node.ShardCoordinator.SelfId()) + nodeRootHash, _ := node.AccntState.RootHash() + assert.Equal(t, proposerRootHash, nodeRootHash) + } +} diff --git a/process/smartContract/process.go b/process/smartContract/process.go index 4556380d23d..8d10d9d851c 100644 --- a/process/smartContract/process.go +++ b/process/smartContract/process.go @@ -403,6 +403,19 @@ func (sc *scProcessor) processVMOutput( return nil, err } + //we need to reload the sender account as in the case of refunding the exact account that was previously + //modified in saveSCOutputToCurrentState, the modifications done there should be visible here + //this should be applied only on sender accounts from current shard + if acntSnd != nil && !acntSnd.IsInterfaceNil() { + isAccountFromCurrentShard := acntSnd.AddressContainer() != nil + if isAccountFromCurrentShard { + acntSnd, err = sc.getAccountFromAddress(acntSnd.AddressContainer().Bytes()) + if err != nil { + return nil, err + } + } + } + totalGasRefund := big.NewInt(0) totalGasRefund = totalGasRefund.Add(vmOutput.GasRefund, vmOutput.GasRemaining) scrIfCrossShard, err := sc.refundGasToSender(totalGasRefund, tx, txHash, acntSnd) diff --git a/process/smartContract/process_test.go b/process/smartContract/process_test.go index dd1f9466961..735e4f7f8cd 100644 --- a/process/smartContract/process_test.go +++ b/process/smartContract/process_test.go @@ -514,12 +514,13 @@ func TestScProcessor_DeploySmartContract(t *testing.T) { addrConverter := &mock.AddressConverterMock{} vm := &mock.VMExecutionHandlerStub{} argParser := &mock.ArgumentParserMock{} + accntState := &mock.AccountsStub{} sc, err := NewSmartContractProcessor( vm, argParser, &mock.HasherMock{}, &mock.MarshalizerMock{}, - &mock.AccountsStub{}, + accntState, &mock.TemporaryAccountsHandlerMock{}, addrConverter, mock.NewMultiShardsCoordinatorMock(5), @@ -535,6 +536,10 @@ func TestScProcessor_DeploySmartContract(t *testing.T) { tx.Value = big.NewInt(45) acntSrc, _ := createAccounts(tx) + accntState.GetAccountWithJournalCalled = func(addressContainer state.AddressContainer) (handler state.AccountHandler, e error) { + return acntSrc, nil + } + err = sc.DeploySmartContract(tx, acntSrc, 10) assert.Equal(t, nil, err) } @@ -682,12 +687,13 @@ func TestScProcessor_ExecuteSmartContractTransaction(t *testing.T) { vm := &mock.VMExecutionHandlerStub{} argParser := &mock.ArgumentParserMock{} + accntState := &mock.AccountsStub{} sc, err := NewSmartContractProcessor( vm, argParser, &mock.HasherMock{}, &mock.MarshalizerMock{}, - &mock.AccountsStub{}, + accntState, &mock.TemporaryAccountsHandlerMock{}, &mock.AddressConverterMock{}, mock.NewMultiShardsCoordinatorMock(5), @@ -703,6 +709,10 @@ func TestScProcessor_ExecuteSmartContractTransaction(t *testing.T) { tx.Value = big.NewInt(45) acntSrc, acntDst := createAccounts(tx) + accntState.GetAccountWithJournalCalled = func(addressContainer state.AddressContainer) (handler state.AccountHandler, e error) { + return acntSrc, nil + } + acntDst.SetCode([]byte("code")) err = sc.ExecuteSmartContractTransaction(tx, acntSrc, acntDst, 10) assert.Nil(t, err) @@ -996,12 +1006,13 @@ func TestScProcessor_processVMOutputNilDstAcc(t *testing.T) { vm := &mock.VMExecutionHandlerStub{} argParser := &mock.ArgumentParserMock{} + accntState := &mock.AccountsStub{} sc, err := NewSmartContractProcessor( vm, argParser, &mock.HasherMock{}, &mock.MarshalizerMock{}, - &mock.AccountsStub{}, + accntState, &mock.TemporaryAccountsHandlerMock{}, &mock.AddressConverterMock{}, mock.NewMultiShardsCoordinatorMock(5), @@ -1015,6 +1026,11 @@ func TestScProcessor_processVMOutputNilDstAcc(t *testing.T) { GasRefund: big.NewInt(0), GasRemaining: big.NewInt(0), } + + accntState.GetAccountWithJournalCalled = func(addressContainer state.AddressContainer) (handler state.AccountHandler, e error) { + return acntSnd, nil + } + _, err = sc.processVMOutput(vmOutput, tx, acntSnd, 10) assert.Nil(t, err) } @@ -1643,12 +1659,13 @@ func TestScProcessor_processVMOutput(t *testing.T) { round := uint32(10) acntSrc, _, tx := createAccountsAndTransaction() + accntState := &mock.AccountsStub{} sc, err := NewSmartContractProcessor( &mock.VMExecutionHandlerStub{}, &mock.ArgumentParserMock{}, &mock.HasherMock{}, &mock.MarshalizerMock{}, - &mock.AccountsStub{}, + accntState, &mock.TemporaryAccountsHandlerMock{}, &mock.AddressConverterMock{}, mock.NewMultiShardsCoordinatorMock(5), @@ -1660,6 +1677,11 @@ func TestScProcessor_processVMOutput(t *testing.T) { GasRefund: big.NewInt(0), GasRemaining: big.NewInt(0), } + + accntState.GetAccountWithJournalCalled = func(addressContainer state.AddressContainer) (handler state.AccountHandler, e error) { + return acntSrc, nil + } + _, err = sc.ProcessVMOutput(vmOutput, tx, acntSrc, round) assert.Nil(t, err) } From bc6d4a63ae1bca761e5a4cccb9cd366c672cafbe Mon Sep 17 00:00:00 2001 From: iulianpascalau Date: Mon, 29 Jul 2019 11:28:07 +0300 Subject: [PATCH 2/5] made integration test single shard executing miniblocksSc run on 4 nodes instead of 1 --- .../singleShard/block/executingMiniblocksSc_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integrationTests/singleShard/block/executingMiniblocksSc_test.go b/integrationTests/singleShard/block/executingMiniblocksSc_test.go index faf92133631..c7d3509d057 100644 --- a/integrationTests/singleShard/block/executingMiniblocksSc_test.go +++ b/integrationTests/singleShard/block/executingMiniblocksSc_test.go @@ -32,7 +32,7 @@ func TestShouldProcessBlocksWithScTxsJoinAndRewardTheOwner(t *testing.T) { assert.Nil(t, err) maxShards := uint32(1) - numOfNodes := 1 + numOfNodes := 4 advertiser := integrationTests.CreateMessengerWithKadDht(context.Background(), "") _ = advertiser.Bootstrap() advertiserAddr := integrationTests.GetConnectableAddress(advertiser) From a024a2316377e2144cd9079e6ff7818c5952178e Mon Sep 17 00:00:00 2001 From: iulianpascalau Date: Tue, 30 Jul 2019 13:35:25 +0300 Subject: [PATCH 3/5] removed "step" from methods names in singleShard/executingMiniblockSc_test --- .../block/executingMiniblocksSc_test.go | 50 +++++++++---------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/integrationTests/singleShard/block/executingMiniblocksSc_test.go b/integrationTests/singleShard/block/executingMiniblocksSc_test.go index c7d3509d057..b84df462cd7 100644 --- a/integrationTests/singleShard/block/executingMiniblocksSc_test.go +++ b/integrationTests/singleShard/block/executingMiniblocksSc_test.go @@ -67,19 +67,19 @@ func TestShouldProcessBlocksWithScTxsJoinAndRewardTheOwner(t *testing.T) { initialVal := big.NewInt(10000000) topUpValue := big.NewInt(500) withdrawValue := big.NewInt(10) - stepMintAllNodes(nodes, initialVal) + mintAllNodes(nodes, initialVal) - stepDeployScTx(nodes, idxProposer, string(scCode)) - stepProposeBlock(nodes, idxProposer, round) - stepSyncBlock(t, nodes, idxProposer, round) + deployScTx(nodes, idxProposer, string(scCode)) + proposeBlock(nodes, idxProposer, round) + syncBlock(t, nodes, idxProposer, round) round = incrementAndPrintRound(round) - stepNodeDoesJoinGame(nodes, idxProposer, topUpValue, hardCodedScResultingAddress) - stepProposeBlock(nodes, idxProposer, round) - stepSyncBlock(t, nodes, idxProposer, round) + nodeDoesJoinGame(nodes, idxProposer, topUpValue, hardCodedScResultingAddress) + proposeBlock(nodes, idxProposer, round) + syncBlock(t, nodes, idxProposer, round) round = incrementAndPrintRound(round) - stepCheckJoinGameIsDoneCorrectly( + checkJoinGameIsDoneCorrectly( t, nodes, idxProposer, @@ -89,16 +89,16 @@ func TestShouldProcessBlocksWithScTxsJoinAndRewardTheOwner(t *testing.T) { hardCodedScResultingAddress, ) - stepNodeCallsRewardAndSend(nodes, idxProposer, idxProposer, withdrawValue, hardCodedScResultingAddress) - stepProposeBlock(nodes, idxProposer, round) - stepSyncBlock(t, nodes, idxProposer, round) + nodeCallsRewardAndSend(nodes, idxProposer, idxProposer, withdrawValue, hardCodedScResultingAddress) + proposeBlock(nodes, idxProposer, round) + syncBlock(t, nodes, idxProposer, round) round = incrementAndPrintRound(round) - stepProposeBlock(nodes, idxProposer, round) - stepSyncBlock(t, nodes, idxProposer, round) + proposeBlock(nodes, idxProposer, round) + syncBlock(t, nodes, idxProposer, round) round = incrementAndPrintRound(round) - stepCheckRewardIsDoneCorrectly( + checkRewardIsDoneCorrectly( t, nodes, idxProposer, @@ -109,7 +109,7 @@ func TestShouldProcessBlocksWithScTxsJoinAndRewardTheOwner(t *testing.T) { hardCodedScResultingAddress, ) - stepCheckRootHashes(t, nodes, []int{0}) + checkRootHashes(t, nodes, []int{0}) time.Sleep(1 * time.Second) } @@ -121,7 +121,7 @@ func incrementAndPrintRound(round uint32) uint32 { return round } -func stepMintAllNodes(nodes []*integrationTests.TestProcessorNode, value *big.Int) { +func mintAllNodes(nodes []*integrationTests.TestProcessorNode, value *big.Int) { for _, n := range nodes { if n.ShardCoordinator.SelfId() == sharding.MetachainShardId { continue @@ -133,7 +133,7 @@ func stepMintAllNodes(nodes []*integrationTests.TestProcessorNode, value *big.In } } -func stepDeployScTx(nodes []*integrationTests.TestProcessorNode, senderIdx int, scCode string) { +func deployScTx(nodes []*integrationTests.TestProcessorNode, senderIdx int, scCode string) { fmt.Println("Deploying SC...") txDeploy := createTxDeploy(nodes[senderIdx], scCode) nodes[senderIdx].SendTransaction(txDeploy) @@ -143,14 +143,14 @@ func stepDeployScTx(nodes []*integrationTests.TestProcessorNode, senderIdx int, fmt.Println(integrationTests.MakeDisplayTable(nodes)) } -func stepProposeBlock(nodes []*integrationTests.TestProcessorNode, idxProposer int, round uint32) { +func proposeBlock(nodes []*integrationTests.TestProcessorNode, idxProposer int, round uint32) { fmt.Println("Proposing block...") for idx, n := range nodes { if idx != idxProposer { continue } - body, header := n.ProposeBlockOnlyWithSelf(round) + body, header := n.ProposeBlock(round) n.BroadcastAndCommit(body, header) } @@ -159,7 +159,7 @@ func stepProposeBlock(nodes []*integrationTests.TestProcessorNode, idxProposer i fmt.Println(integrationTests.MakeDisplayTable(nodes)) } -func stepSyncBlock(t *testing.T, nodes []*integrationTests.TestProcessorNode, idxProposer int, round uint32) { +func syncBlock(t *testing.T, nodes []*integrationTests.TestProcessorNode, idxProposer int, round uint32) { fmt.Println("All other shard nodes sync the proposed block...") for idx, n := range nodes { if idx == idxProposer { @@ -177,7 +177,7 @@ func stepSyncBlock(t *testing.T, nodes []*integrationTests.TestProcessorNode, id fmt.Println(integrationTests.MakeDisplayTable(nodes)) } -func stepNodeDoesJoinGame( +func nodeDoesJoinGame( nodes []*integrationTests.TestProcessorNode, idxNode int, joinGameVal *big.Int, @@ -192,7 +192,7 @@ func stepNodeDoesJoinGame( fmt.Println(integrationTests.MakeDisplayTable(nodes)) } -func stepNodeCallsRewardAndSend( +func nodeCallsRewardAndSend( nodes []*integrationTests.TestProcessorNode, idxNodeOwner int, idxNodeUser int, @@ -260,7 +260,7 @@ func createTxRewardAndSendToWallet(tnOwner *integrationTests.TestProcessorNode, return tx } -func stepCheckJoinGameIsDoneCorrectly( +func checkJoinGameIsDoneCorrectly( t *testing.T, nodes []*integrationTests.TestProcessorNode, idxNodeScExists int, @@ -287,7 +287,7 @@ func stepCheckJoinGameIsDoneCorrectly( assert.Equal(t, expectedVal, accnt.(*state.Account).Balance) } -func stepCheckRewardIsDoneCorrectly( +func checkRewardIsDoneCorrectly( t *testing.T, nodes []*integrationTests.TestProcessorNode, idxNodeScExists int, @@ -318,7 +318,7 @@ func stepCheckRewardIsDoneCorrectly( assert.Equal(t, expectedSender, accnt.(*state.Account).Balance) } -func stepCheckRootHashes(t *testing.T, nodes []*integrationTests.TestProcessorNode, idxProposers []int) { +func checkRootHashes(t *testing.T, nodes []*integrationTests.TestProcessorNode, idxProposers []int) { for _, idx := range idxProposers { checkRootHashInShard(t, nodes, idx) } From 58518a904097728d565743c3a4db9a01516f275d Mon Sep 17 00:00:00 2001 From: iulianpascalau Date: Tue, 30 Jul 2019 15:23:56 +0300 Subject: [PATCH 4/5] minor code changes in intgration tests: moved mint all addresses in test initializer, small code reformatting, changed some variable names, removed one extra step (propose-sync) in singleshard/block executingMiniblocksSc --- .../block/executingMiniblocksSc_test.go | 41 +++++-------------- .../block/executingMiniblocksSc_test.go | 35 +++++----------- integrationTests/testInitializer.go | 24 +++++++++++ integrationTests/testProcessorNode.go | 3 ++ 4 files changed, 47 insertions(+), 56 deletions(-) diff --git a/integrationTests/multiShard/block/executingMiniblocksSc_test.go b/integrationTests/multiShard/block/executingMiniblocksSc_test.go index 5e82b6896e5..b64af93acd7 100644 --- a/integrationTests/multiShard/block/executingMiniblocksSc_test.go +++ b/integrationTests/multiShard/block/executingMiniblocksSc_test.go @@ -71,7 +71,7 @@ func TestProcessWithScTxsTopUpAndWithdrawOnlyProposers(t *testing.T) { initialVal := big.NewInt(10000000) topUpValue := big.NewInt(500) withdrawValue := big.NewInt(10) - mintAllNodes(nodes, initialVal) + integrationTests.MintAllNodes(nodes, initialVal) deployScTx(nodes, idxNodeShard1, string(scCode)) @@ -182,7 +182,7 @@ func TestProcessWithScTxsJoinAndRewardTwoNodesInShard(t *testing.T) { initialVal := big.NewInt(10000000) topUpValue := big.NewInt(500) withdrawValue := big.NewInt(10) - mintAllNodes(nodes, initialVal) + integrationTests.MintAllNodes(nodes, initialVal) deployScTx(nodes, idxProposerShard1, string(scCode)) @@ -243,27 +243,6 @@ func incrementAndPrintRound(round uint32) uint32 { return round } -func mintAllNodes( - nodes []*integrationTests.TestProcessorNode, - value *big.Int, -) { - - for _, n := range nodes { - if n.ShardCoordinator.SelfId() == sharding.MetachainShardId { - continue - } - - for _, n2 := range nodes { - addr := integrationTests.CreateAddresFromAddrBytes(n2.PkTxSignBytes) - if n.ShardCoordinator.ComputeId(addr) != n.ShardCoordinator.SelfId() { - continue - } - - integrationTests.MintAddress(n.AccntState, n2.PkTxSignBytes, value) - } - } -} - func deployScTx( nodes []*integrationTests.TestProcessorNode, senderIdx int, @@ -342,8 +321,8 @@ func nodeDoesTopUp( ) { fmt.Println("Calling SC.topUp...") - txDeploy := createTxTopUp(nodes[idxNode], topUpValue, scAddress) - nodes[idxNode].SendTransaction(txDeploy) + txScCall := createTxTopUp(nodes[idxNode], topUpValue, scAddress) + nodes[idxNode].SendTransaction(txScCall) fmt.Println("Delaying for disseminating SC call tx...") time.Sleep(stepDelay) @@ -358,8 +337,8 @@ func nodeDoesJoinGame( ) { fmt.Println("Calling SC.joinGame...") - txDeploy := createTxJoinGame(nodes[idxNode], joinGameVal, scAddress) - nodes[idxNode].SendTransaction(txDeploy) + txScCall := createTxJoinGame(nodes[idxNode], joinGameVal, scAddress) + nodes[idxNode].SendTransaction(txScCall) fmt.Println("Delaying for disseminating SC call tx...") time.Sleep(stepDelay) @@ -436,8 +415,8 @@ func nodeDoesWithdraw( ) { fmt.Println("Calling SC.withdraw...") - txDeploy := createTxWithdraw(nodes[idxNode], withdrawValue, scAddress) - nodes[idxNode].SendTransaction(txDeploy) + txScCall := createTxWithdraw(nodes[idxNode], withdrawValue, scAddress) + nodes[idxNode].SendTransaction(txScCall) fmt.Println("Delaying for disseminating SC call tx...") time.Sleep(time.Second * 1) @@ -453,8 +432,8 @@ func nodeCallsRewardAndSend( ) { fmt.Println("Calling SC.rewardAndSendToWallet...") - txDeploy := createTxRewardAndSendToWallet(nodes[idxNodeOwner], nodes[idxNodeUser], prize, scAddress) - nodes[idxNodeOwner].SendTransaction(txDeploy) + txScCall := createTxRewardAndSendToWallet(nodes[idxNodeOwner], nodes[idxNodeUser], prize, scAddress) + nodes[idxNodeOwner].SendTransaction(txScCall) fmt.Println("Delaying for disseminating SC call tx...") time.Sleep(time.Second * 1) diff --git a/integrationTests/singleShard/block/executingMiniblocksSc_test.go b/integrationTests/singleShard/block/executingMiniblocksSc_test.go index b84df462cd7..a8fb304f422 100644 --- a/integrationTests/singleShard/block/executingMiniblocksSc_test.go +++ b/integrationTests/singleShard/block/executingMiniblocksSc_test.go @@ -13,14 +13,13 @@ import ( "github.com/ElrondNetwork/elrond-go/data/state" "github.com/ElrondNetwork/elrond-go/data/transaction" "github.com/ElrondNetwork/elrond-go/integrationTests" - "github.com/ElrondNetwork/elrond-go/sharding" "github.com/stretchr/testify/assert" ) var agarioFile = "agarioV2.hex" var stepDelay = time.Second -func TestShouldProcessBlocksWithScTxsJoinAndRewardTheOwner(t *testing.T) { +func TestShouldProcessWithScTxsJoinAndRewardTheOwner(t *testing.T) { if testing.Short() { t.Skip("this is not a short test") } @@ -67,7 +66,7 @@ func TestShouldProcessBlocksWithScTxsJoinAndRewardTheOwner(t *testing.T) { initialVal := big.NewInt(10000000) topUpValue := big.NewInt(500) withdrawValue := big.NewInt(10) - mintAllNodes(nodes, initialVal) + integrationTests.MintAllNodes(nodes, initialVal) deployScTx(nodes, idxProposer, string(scCode)) proposeBlock(nodes, idxProposer, round) @@ -94,10 +93,6 @@ func TestShouldProcessBlocksWithScTxsJoinAndRewardTheOwner(t *testing.T) { syncBlock(t, nodes, idxProposer, round) round = incrementAndPrintRound(round) - proposeBlock(nodes, idxProposer, round) - syncBlock(t, nodes, idxProposer, round) - round = incrementAndPrintRound(round) - checkRewardIsDoneCorrectly( t, nodes, @@ -121,18 +116,6 @@ func incrementAndPrintRound(round uint32) uint32 { return round } -func mintAllNodes(nodes []*integrationTests.TestProcessorNode, value *big.Int) { - for _, n := range nodes { - if n.ShardCoordinator.SelfId() == sharding.MetachainShardId { - continue - } - - for _, n2 := range nodes { - integrationTests.MintAddress(n.AccntState, n2.PkTxSignBytes, value) - } - } -} - func deployScTx(nodes []*integrationTests.TestProcessorNode, senderIdx int, scCode string) { fmt.Println("Deploying SC...") txDeploy := createTxDeploy(nodes[senderIdx], scCode) @@ -181,11 +164,12 @@ func nodeDoesJoinGame( nodes []*integrationTests.TestProcessorNode, idxNode int, joinGameVal *big.Int, - scAddress []byte) { + scAddress []byte, +) { fmt.Println("Calling SC.joinGame...") - txDeploy := createTxJoinGame(nodes[idxNode], joinGameVal, scAddress) - nodes[idxNode].SendTransaction(txDeploy) + txScCall := createTxJoinGame(nodes[idxNode], joinGameVal, scAddress) + nodes[idxNode].SendTransaction(txScCall) fmt.Println("Delaying for disseminating SC call tx...") time.Sleep(stepDelay) @@ -197,11 +181,12 @@ func nodeCallsRewardAndSend( idxNodeOwner int, idxNodeUser int, prize *big.Int, - scAddress []byte) { + scAddress []byte, +) { fmt.Println("Calling SC.rewardAndSendToWallet...") - txDeploy := createTxRewardAndSendToWallet(nodes[idxNodeOwner], nodes[idxNodeUser], prize, scAddress) - nodes[idxNodeOwner].SendTransaction(txDeploy) + txScCall := createTxRewardAndSendToWallet(nodes[idxNodeOwner], nodes[idxNodeUser], prize, scAddress) + nodes[idxNodeOwner].SendTransaction(txScCall) fmt.Println("Delaying for disseminating SC call tx...") time.Sleep(time.Second * 1) diff --git a/integrationTests/testInitializer.go b/integrationTests/testInitializer.go index e56c01efeb5..e60cb420f83 100644 --- a/integrationTests/testInitializer.go +++ b/integrationTests/testInitializer.go @@ -310,3 +310,27 @@ func PrintShardAccount(accnt *state.Account) { fmt.Println(str) } + +// MintAllNodes will take each shard node (n) and will mint all nodes that have their pk managed by the iterating node n +func MintAllNodes(nodes []*TestProcessorNode, value *big.Int) { + for idx, n := range nodes { + if n.ShardCoordinator.SelfId() == sharding.MetachainShardId { + continue + } + + mintAddressesFromSameShard(nodes, idx, value) + } +} + +func mintAddressesFromSameShard(nodes []*TestProcessorNode, targetNodeIdx int, value *big.Int) { + targetNode := nodes[targetNodeIdx] + + for _, n := range nodes { + shardId := targetNode.ShardCoordinator.ComputeId(n.TxSignAddress) + if shardId != targetNode.ShardCoordinator.SelfId() { + continue + } + + MintAddress(targetNode.AccntState, n.PkTxSignBytes, value) + } +} diff --git a/integrationTests/testProcessorNode.go b/integrationTests/testProcessorNode.go index e368ee4f09f..da95e5857cb 100644 --- a/integrationTests/testProcessorNode.go +++ b/integrationTests/testProcessorNode.go @@ -72,6 +72,7 @@ type TestProcessorNode struct { PkTxSign crypto.PublicKey PkTxSignBytes []byte KeygenTxSign crypto.KeyGenerator + TxSignAddress state.AddressContainer ShardDataPool dataRetriever.PoolsHolder MetaDataPool dataRetriever.MetaPoolsHolder @@ -168,6 +169,7 @@ func (tpn *TestProcessorNode) initCrypto(txSignPrivKeyShardId uint32) { tpn.PkTxSign = pk tpn.PkTxSignBytes, _ = pk.ToByteArray() tpn.KeygenTxSign = keyGen + tpn.TxSignAddress, _ = TestAddressConverter.CreateAddressFromPublicKeyBytes(tpn.PkTxSignBytes) } func (tpn *TestProcessorNode) initDataPools() { @@ -506,6 +508,7 @@ func (tpn *TestProcessorNode) LoadTxSignSkBytes(skBytes []byte) { tpn.SkTxSign = newSk tpn.PkTxSign = newPk tpn.PkTxSignBytes, _ = newPk.ToByteArray() + tpn.TxSignAddress, _ = TestAddressConverter.CreateAddressFromPublicKeyBytes(tpn.PkTxSignBytes) } // ProposeBlock proposes a new block From b9c76e0b6decebbaff9428bb3b3686b3b4abd133 Mon Sep 17 00:00:00 2001 From: iulianpascalau Date: Tue, 30 Jul 2019 16:55:16 +0300 Subject: [PATCH 5/5] function renaming (nodeDoesJoinGame -> nodeJoinsGame), moved the fix located in smartcontract/process in a function called reloadLocalSndAccount --- .../block/executingMiniblocksSc_test.go | 4 +-- .../block/executingMiniblocksSc_test.go | 4 +-- process/smartContract/process.go | 30 ++++++++++++------- 3 files changed, 23 insertions(+), 15 deletions(-) diff --git a/integrationTests/multiShard/block/executingMiniblocksSc_test.go b/integrationTests/multiShard/block/executingMiniblocksSc_test.go index b64af93acd7..bd16b5976ba 100644 --- a/integrationTests/multiShard/block/executingMiniblocksSc_test.go +++ b/integrationTests/multiShard/block/executingMiniblocksSc_test.go @@ -190,7 +190,7 @@ func TestProcessWithScTxsJoinAndRewardTwoNodesInShard(t *testing.T) { syncBlock(t, nodes, idxProposers, round) round = incrementAndPrintRound(round) - nodeDoesJoinGame(nodes, idxProposerShard0, topUpValue, hardCodedScResultingAddress) + nodeJoinsGame(nodes, idxProposerShard0, topUpValue, hardCodedScResultingAddress) roundsToWait := 6 for i := 0; i < roundsToWait; i++ { @@ -329,7 +329,7 @@ func nodeDoesTopUp( fmt.Println(integrationTests.MakeDisplayTable(nodes)) } -func nodeDoesJoinGame( +func nodeJoinsGame( nodes []*integrationTests.TestProcessorNode, idxNode int, joinGameVal *big.Int, diff --git a/integrationTests/singleShard/block/executingMiniblocksSc_test.go b/integrationTests/singleShard/block/executingMiniblocksSc_test.go index a8fb304f422..827164658f9 100644 --- a/integrationTests/singleShard/block/executingMiniblocksSc_test.go +++ b/integrationTests/singleShard/block/executingMiniblocksSc_test.go @@ -73,7 +73,7 @@ func TestShouldProcessWithScTxsJoinAndRewardTheOwner(t *testing.T) { syncBlock(t, nodes, idxProposer, round) round = incrementAndPrintRound(round) - nodeDoesJoinGame(nodes, idxProposer, topUpValue, hardCodedScResultingAddress) + nodeJoinsGame(nodes, idxProposer, topUpValue, hardCodedScResultingAddress) proposeBlock(nodes, idxProposer, round) syncBlock(t, nodes, idxProposer, round) round = incrementAndPrintRound(round) @@ -160,7 +160,7 @@ func syncBlock(t *testing.T, nodes []*integrationTests.TestProcessorNode, idxPro fmt.Println(integrationTests.MakeDisplayTable(nodes)) } -func nodeDoesJoinGame( +func nodeJoinsGame( nodes []*integrationTests.TestProcessorNode, idxNode int, joinGameVal *big.Int, diff --git a/process/smartContract/process.go b/process/smartContract/process.go index 8d10d9d851c..43e222ab135 100644 --- a/process/smartContract/process.go +++ b/process/smartContract/process.go @@ -403,17 +403,9 @@ func (sc *scProcessor) processVMOutput( return nil, err } - //we need to reload the sender account as in the case of refunding the exact account that was previously - //modified in saveSCOutputToCurrentState, the modifications done there should be visible here - //this should be applied only on sender accounts from current shard - if acntSnd != nil && !acntSnd.IsInterfaceNil() { - isAccountFromCurrentShard := acntSnd.AddressContainer() != nil - if isAccountFromCurrentShard { - acntSnd, err = sc.getAccountFromAddress(acntSnd.AddressContainer().Bytes()) - if err != nil { - return nil, err - } - } + acntSnd, err = sc.reloadLocalSndAccount(acntSnd) + if err != nil { + return nil, err } totalGasRefund := big.NewInt(0) @@ -440,6 +432,22 @@ func (sc *scProcessor) processVMOutput( return crossTxs, nil } +// reloadLocalSndAccount will reload from current account state the sender account +// this requirement is needed because in the case of refunding the exact account that was previously +// modified in saveSCOutputToCurrentState, the modifications done there should be visible here +func (sc *scProcessor) reloadLocalSndAccount(acntSnd state.AccountHandler) (state.AccountHandler, error) { + if acntSnd == nil || acntSnd.IsInterfaceNil() { + return acntSnd, nil + } + + isAccountFromCurrentShard := acntSnd.AddressContainer() != nil + if !isAccountFromCurrentShard { + return acntSnd, nil + } + + return sc.getAccountFromAddress(acntSnd.AddressContainer().Bytes()) +} + func (sc *scProcessor) createSmartContractResult( outAcc *vmcommon.OutputAccount, scAddress []byte,