From a308f012ba6bac3249ae52727e1c52273390d7e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Szil=C3=A1gyi?= Date: Tue, 24 Sep 2019 10:49:59 +0300 Subject: [PATCH] core/state: fix copy-commit-copy (#20113) * core/state: revert noop finalise, fix copy-commit-copy * core/state: reintroduce net sstore tracking, extend tests for it --- core/state/statedb.go | 7 +- core/state/statedb_test.go | 165 ++++++++++++++++++++++++++++++++++++- 2 files changed, 167 insertions(+), 5 deletions(-) diff --git a/core/state/statedb.go b/core/state/statedb.go index 9034d5533..4b4f374c9 100644 --- a/core/state/statedb.go +++ b/core/state/statedb.go @@ -588,8 +588,13 @@ func (self *StateDB) Copy() *StateDB { // in the stateObjects: OOG after touch on ripeMD prior to Byzantium. Thus, we need to check for // nil if object, exist := self.stateObjects[addr]; exist { + // Even though the original object is dirty, we are not copying the journal, + // so we need to make sure that anyside effect the journal would have caused + // during a commit (or similar op) is already applied to the copy. state.stateObjects[addr] = object.deepCopy(state) - state.stateObjectsDirty[addr] = struct{}{} + + state.stateObjectsDirty[addr] = struct{}{} // Mark the copy dirty to force internal (code/state) commits + state.stateObjectsPending[addr] = struct{}{} // Mark the copy pending to force external (account) commits } } // Above, we don't copy the actual journal. This means that if the copy is copied, the diff --git a/core/state/statedb_test.go b/core/state/statedb_test.go index bb3b87ead..5f58a234c 100644 --- a/core/state/statedb_test.go +++ b/core/state/statedb_test.go @@ -438,18 +438,175 @@ func (s *StateSuite) TestTouchDelete(c *check.C) { // TestCopyOfCopy tests that modified objects are carried over to the copy, and the copy of the copy. // See https://github.com/ethereum/go-ethereum/pull/15225#issuecomment-380191512 func TestCopyOfCopy(t *testing.T) { - sdb, _ := New(common.Hash{}, NewDatabase(rawdb.NewMemoryDatabase())) + state, _ := New(common.Hash{}, NewDatabase(rawdb.NewMemoryDatabase())) addr := common.HexToAddress("aaaa") - sdb.SetBalance(addr, big.NewInt(42)) + state.SetBalance(addr, big.NewInt(42)) - if got := sdb.Copy().GetBalance(addr).Uint64(); got != 42 { + if got := state.Copy().GetBalance(addr).Uint64(); got != 42 { t.Fatalf("1st copy fail, expected 42, got %v", got) } - if got := sdb.Copy().Copy().GetBalance(addr).Uint64(); got != 42 { + if got := state.Copy().Copy().GetBalance(addr).Uint64(); got != 42 { t.Fatalf("2nd copy fail, expected 42, got %v", got) } } +// Tests a regression where committing a copy lost some internal meta information, +// leading to corrupted subsequent copies. +// +// See https://github.com/ethereum/go-ethereum/issues/20106. +func TestCopyCommitCopy(t *testing.T) { + state, _ := New(common.Hash{}, NewDatabase(rawdb.NewMemoryDatabase())) + + // Create an account and check if the retrieved balance is correct + addr := common.HexToAddress("0xaffeaffeaffeaffeaffeaffeaffeaffeaffeaffe") + skey := common.HexToHash("aaa") + sval := common.HexToHash("bbb") + + state.SetBalance(addr, big.NewInt(42)) // Change the account trie + state.SetCode(addr, []byte("hello")) // Change an external metadata + state.SetState(addr, skey, sval) // Change the storage trie + + if balance := state.GetBalance(addr); balance.Cmp(big.NewInt(42)) != 0 { + t.Fatalf("initial balance mismatch: have %v, want %v", balance, 42) + } + if code := state.GetCode(addr); !bytes.Equal(code, []byte("hello")) { + t.Fatalf("initial code mismatch: have %x, want %x", code, []byte("hello")) + } + if val := state.GetState(addr, skey); val != sval { + t.Fatalf("initial non-committed storage slot mismatch: have %x, want %x", val, sval) + } + if val := state.GetCommittedState(addr, skey); val != (common.Hash{}) { + t.Fatalf("initial committed storage slot mismatch: have %x, want %x", val, common.Hash{}) + } + // Copy the non-committed state database and check pre/post commit balance + copyOne := state.Copy() + if balance := copyOne.GetBalance(addr); balance.Cmp(big.NewInt(42)) != 0 { + t.Fatalf("first copy pre-commit balance mismatch: have %v, want %v", balance, 42) + } + if code := copyOne.GetCode(addr); !bytes.Equal(code, []byte("hello")) { + t.Fatalf("first copy pre-commit code mismatch: have %x, want %x", code, []byte("hello")) + } + if val := copyOne.GetState(addr, skey); val != sval { + t.Fatalf("first copy pre-commit non-committed storage slot mismatch: have %x, want %x", val, sval) + } + if val := copyOne.GetCommittedState(addr, skey); val != (common.Hash{}) { + t.Fatalf("first copy pre-commit committed storage slot mismatch: have %x, want %x", val, common.Hash{}) + } + + copyOne.Commit(false) + if balance := copyOne.GetBalance(addr); balance.Cmp(big.NewInt(42)) != 0 { + t.Fatalf("first copy post-commit balance mismatch: have %v, want %v", balance, 42) + } + if code := copyOne.GetCode(addr); !bytes.Equal(code, []byte("hello")) { + t.Fatalf("first copy post-commit code mismatch: have %x, want %x", code, []byte("hello")) + } + if val := copyOne.GetState(addr, skey); val != sval { + t.Fatalf("first copy post-commit non-committed storage slot mismatch: have %x, want %x", val, sval) + } + if val := copyOne.GetCommittedState(addr, skey); val != sval { + t.Fatalf("first copy post-commit committed storage slot mismatch: have %x, want %x", val, sval) + } + // Copy the copy and check the balance once more + copyTwo := copyOne.Copy() + if balance := copyTwo.GetBalance(addr); balance.Cmp(big.NewInt(42)) != 0 { + t.Fatalf("second copy balance mismatch: have %v, want %v", balance, 42) + } + if code := copyTwo.GetCode(addr); !bytes.Equal(code, []byte("hello")) { + t.Fatalf("second copy code mismatch: have %x, want %x", code, []byte("hello")) + } + if val := copyTwo.GetState(addr, skey); val != sval { + t.Fatalf("second copy non-committed storage slot mismatch: have %x, want %x", val, sval) + } + if val := copyTwo.GetCommittedState(addr, skey); val != sval { + t.Fatalf("second copy post-commit committed storage slot mismatch: have %x, want %x", val, sval) + } +} + +// Tests a regression where committing a copy lost some internal meta information, +// leading to corrupted subsequent copies. +// +// See https://github.com/ethereum/go-ethereum/issues/20106. +func TestCopyCopyCommitCopy(t *testing.T) { + state, _ := New(common.Hash{}, NewDatabase(rawdb.NewMemoryDatabase())) + + // Create an account and check if the retrieved balance is correct + addr := common.HexToAddress("0xaffeaffeaffeaffeaffeaffeaffeaffeaffeaffe") + skey := common.HexToHash("aaa") + sval := common.HexToHash("bbb") + + state.SetBalance(addr, big.NewInt(42)) // Change the account trie + state.SetCode(addr, []byte("hello")) // Change an external metadata + state.SetState(addr, skey, sval) // Change the storage trie + + if balance := state.GetBalance(addr); balance.Cmp(big.NewInt(42)) != 0 { + t.Fatalf("initial balance mismatch: have %v, want %v", balance, 42) + } + if code := state.GetCode(addr); !bytes.Equal(code, []byte("hello")) { + t.Fatalf("initial code mismatch: have %x, want %x", code, []byte("hello")) + } + if val := state.GetState(addr, skey); val != sval { + t.Fatalf("initial non-committed storage slot mismatch: have %x, want %x", val, sval) + } + if val := state.GetCommittedState(addr, skey); val != (common.Hash{}) { + t.Fatalf("initial committed storage slot mismatch: have %x, want %x", val, common.Hash{}) + } + // Copy the non-committed state database and check pre/post commit balance + copyOne := state.Copy() + if balance := copyOne.GetBalance(addr); balance.Cmp(big.NewInt(42)) != 0 { + t.Fatalf("first copy balance mismatch: have %v, want %v", balance, 42) + } + if code := copyOne.GetCode(addr); !bytes.Equal(code, []byte("hello")) { + t.Fatalf("first copy code mismatch: have %x, want %x", code, []byte("hello")) + } + if val := copyOne.GetState(addr, skey); val != sval { + t.Fatalf("first copy non-committed storage slot mismatch: have %x, want %x", val, sval) + } + if val := copyOne.GetCommittedState(addr, skey); val != (common.Hash{}) { + t.Fatalf("first copy committed storage slot mismatch: have %x, want %x", val, common.Hash{}) + } + // Copy the copy and check the balance once more + copyTwo := copyOne.Copy() + if balance := copyTwo.GetBalance(addr); balance.Cmp(big.NewInt(42)) != 0 { + t.Fatalf("second copy pre-commit balance mismatch: have %v, want %v", balance, 42) + } + if code := copyTwo.GetCode(addr); !bytes.Equal(code, []byte("hello")) { + t.Fatalf("second copy pre-commit code mismatch: have %x, want %x", code, []byte("hello")) + } + if val := copyTwo.GetState(addr, skey); val != sval { + t.Fatalf("second copy pre-commit non-committed storage slot mismatch: have %x, want %x", val, sval) + } + if val := copyTwo.GetCommittedState(addr, skey); val != (common.Hash{}) { + t.Fatalf("second copy pre-commit committed storage slot mismatch: have %x, want %x", val, common.Hash{}) + } + copyTwo.Commit(false) + if balance := copyTwo.GetBalance(addr); balance.Cmp(big.NewInt(42)) != 0 { + t.Fatalf("second copy post-commit balance mismatch: have %v, want %v", balance, 42) + } + if code := copyTwo.GetCode(addr); !bytes.Equal(code, []byte("hello")) { + t.Fatalf("second copy post-commit code mismatch: have %x, want %x", code, []byte("hello")) + } + if val := copyTwo.GetState(addr, skey); val != sval { + t.Fatalf("second copy post-commit non-committed storage slot mismatch: have %x, want %x", val, sval) + } + if val := copyTwo.GetCommittedState(addr, skey); val != sval { + t.Fatalf("second copy post-commit committed storage slot mismatch: have %x, want %x", val, sval) + } + // Copy the copy-copy and check the balance once more + copyThree := copyTwo.Copy() + if balance := copyThree.GetBalance(addr); balance.Cmp(big.NewInt(42)) != 0 { + t.Fatalf("third copy balance mismatch: have %v, want %v", balance, 42) + } + if code := copyThree.GetCode(addr); !bytes.Equal(code, []byte("hello")) { + t.Fatalf("third copy code mismatch: have %x, want %x", code, []byte("hello")) + } + if val := copyThree.GetState(addr, skey); val != sval { + t.Fatalf("third copy non-committed storage slot mismatch: have %x, want %x", val, sval) + } + if val := copyThree.GetCommittedState(addr, skey); val != sval { + t.Fatalf("third copy committed storage slot mismatch: have %x, want %x", val, sval) + } +} + // TestDeleteCreateRevert tests a weird state transition corner case that we hit // while changing the internals of statedb. The workflow is that a contract is // self destructed, then in a followup transaction (but same block) it's created