From ea3460b425445779493fc8f0462441f5d7975083 Mon Sep 17 00:00:00 2001 From: andyzhang2023 Date: Wed, 23 Oct 2024 10:27:00 +0800 Subject: [PATCH 1/7] fix: Transient Storage should set its prev value but not delete it when reverting --- core/state/pevm_journal.go | 9 +-------- core/state/pevm_statedb.go | 6 +++++- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/core/state/pevm_journal.go b/core/state/pevm_journal.go index 397dc44360..21487449a2 100644 --- a/core/state/pevm_journal.go +++ b/core/state/pevm_journal.go @@ -292,14 +292,7 @@ func newJTransientStorage(addr common.Address, key, val common.Hash) *jTransient } func (j *jTransientStorage) revert(db *UncommittedDB) { - storage, ok := db.transientStorage[j.addr] - if !ok { - return - } - delete(storage, j.key) - if len(storage) == 0 { - delete(db.transientStorage, j.addr) - } + db.transientStorage.Set(j.addr, j.key, j.val) } type ujournal []ustate diff --git a/core/state/pevm_statedb.go b/core/state/pevm_statedb.go index 80bdb712dd..b4a1d8b01a 100644 --- a/core/state/pevm_statedb.go +++ b/core/state/pevm_statedb.go @@ -316,7 +316,11 @@ func (pst *UncommittedDB) GetTransientState(addr common.Address, key common.Hash return pst.transientStorage.Get(addr, key) } func (pst *UncommittedDB) SetTransientState(addr common.Address, key, value common.Hash) { - pst.journal.append(newJTransientStorage(addr, key, value)) + prev := pst.transientStorage.Get(addr, key) + if prev == value { + return // no need to record the same value + } + pst.journal.append(newJTransientStorage(addr, key, prev)) pst.transientStorage.Set(addr, key, value) } From 3af645baf161f754cf14868917efb659147f180f Mon Sep 17 00:00:00 2001 From: andyzhang2023 Date: Fri, 25 Oct 2024 09:12:12 +0800 Subject: [PATCH 2/7] bugfix: it panic when accessing storage from a reverted object which had been destructed --- core/state/pevm_journal.go | 12 +--- core/state/pevm_statedb_test.go | 107 ++++++++++++++++++++++++++++++++ 2 files changed, 108 insertions(+), 11 deletions(-) diff --git a/core/state/pevm_journal.go b/core/state/pevm_journal.go index 21487449a2..a84349b94e 100644 --- a/core/state/pevm_journal.go +++ b/core/state/pevm_journal.go @@ -188,17 +188,7 @@ func newJSelfDestruct(obj *state) *jSelfDestruct { } return &jSelfDestruct{ addr: obj.addr, - obj: &state{ - modified: obj.modified, - addr: obj.addr, - balance: new(uint256.Int).Set(obj.balance), - nonce: obj.nonce, - code: append([]byte(nil), obj.code...), - codeHash: append([]byte(nil), obj.codeHash...), - codeSize: obj.codeSize, - created: obj.created, - deleted: obj.deleted, - }, + obj: obj.clone(), } } diff --git a/core/state/pevm_statedb_test.go b/core/state/pevm_statedb_test.go index 38a917e2de..ebddc22422 100644 --- a/core/state/pevm_statedb_test.go +++ b/core/state/pevm_statedb_test.go @@ -799,6 +799,113 @@ func TestPevmSelfDestruct(t *testing.T) { } } +func TestPevmSelfDestructAndRevert(t *testing.T) { + shadow := newStateDB() + uncommitted := newUncommittedDB(newStateDB()) + //prepare: create an account, set state, set balance + // A1{key1: val1, balance: 100, accesslist: 0x33} + prepare := Tx{ + {"Create", Address1}, + {"SetState", Address1, "key1", "val1"}, + {"AddBalance", Address1, big.NewInt(100)}, + {"AddSlots", Address1, common.Hash{0x33}}, + } + prepareCheck := Checks{ + {"balance", Address1, big.NewInt(100)}, + {"state", Address1, "key1", "val1"}, + {"slot", Address1, common.Hash{0x33}, true}, + } + check := func(verify Checks, shadow, uncommitted vm.StateDB) { + if err := verify.Verify(shadow); err != nil { + t.Fatalf("maindb verify failed, err=%s", err.Error()) + } + if err := verify.Verify(uncommitted); err != nil { + t.Fatalf("uncommitted verify failed, err=%s", err.Error()) + } + } + prepare.Call(shadow) + prepare.Call(uncommitted) + shadow.Finalise(true) + uncommitted.Finalise(true) + check(prepareCheck, shadow, uncommitted) + + // do the following operations, and then selfdestruct, and then revert + // 1. add balance 200, + // 2. set state key1: val2 + // 3. add slot 0x34 + // 4. selfdestruct + // 5. revert + case1 := Tx{ + {"AddBalance", Address1, big.NewInt(200)}, + {"SetState", Address1, "key1", "val2"}, + {"SetState", Address1, "key2", "val0"}, + {"AddSlots", Address1, common.Hash{0x34}}, + } + case1SelfDestruct := Tx{ + {"SelfDestruct", Address1}, + } + beforeSelfDestruct := Checks{ + {"balance", Address1, big.NewInt(300)}, + {"state", Address1, "key1", "val2"}, + {"state", Address1, "key2", "val0"}, + {"slot", Address1, common.Hash{0x34}, true}, + {"slot", Address1, common.Hash{0x33}, true}, + } + beforeRevert := Checks{ + {"balance", Address1, big.NewInt(0)}, + {"state", Address1, "key1", "val2"}, + {"state", Address1, "key2", "val0"}, + {"slot", Address1, common.Hash{0x34}, true}, + {"slot", Address1, common.Hash{0x33}, true}, + } + afterRevert := Checks{ + {"balance", Address1, big.NewInt(100)}, + {"state", Address1, "key1", "val1"}, + {"state", Address1, "key2", ""}, + {"slot", Address1, common.Hash{0x34}, false}, + {"slot", Address1, common.Hash{0x33}, true}, + } + // run the case1 on shadow + snapshot := shadow.Snapshot() + case1.Call(shadow) + if err := beforeSelfDestruct.Verify(shadow); err != nil { + t.Fatalf("maindb ut failed, err:%s", err.Error()) + } + case1SelfDestruct.Call(shadow) + if err := beforeRevert.Verify(shadow); err != nil { + t.Fatalf("maindb ut failed, err:%s", err.Error()) + } + shadow.RevertToSnapshot(snapshot) + if err := afterRevert.Verify(shadow); err != nil { + t.Fatalf("maindb ut failed, err:%s", err.Error()) + } + + // now on uncommitted + snapshot = uncommitted.Snapshot() + case1.Call(uncommitted) + if err := beforeSelfDestruct.Verify(uncommitted); err != nil { + t.Fatalf("uncommitted ut failed, err:%s", err.Error()) + } + case1SelfDestruct.Call(uncommitted) + if err := beforeRevert.Verify(uncommitted); err != nil { + t.Fatalf("uncommitted ut failed, err:%s", err.Error()) + } + uncommitted.RevertToSnapshot(snapshot) + if err := afterRevert.Verify(uncommitted); err != nil { + t.Fatalf("uncommitted ut failed, err:%s", err.Error()) + } + + // now compare the mercle root of shadow and uncommitted + shadow.Finalise(true) + if err := uncommitted.Merge(true); err != nil { + t.Fatalf("ut failed, err:%s", err.Error()) + } + uncommitted.Finalise(true) + if shadow.IntermediateRoot(true) != uncommitted.maindb.IntermediateRoot(true) { + t.Fatalf("ut failed, err: the mercle root of shadow and uncommitted are different") + } +} + func TestPevmSelfDestruct6780(t *testing.T) { // case 1. no previous state txs := Txs{ From 92b23177aa68f0fa96283b66f828804b4c5df26a Mon Sep 17 00:00:00 2001 From: andyzhang2023 Date: Fri, 25 Oct 2024 09:16:18 +0800 Subject: [PATCH 3/7] bugfix: 'invalid mercle root' after reverting an object which had been selfdestruct6780 --- core/state/pevm_statedb.go | 2 +- core/state/pevm_statedb_test.go | 105 ++++++++++++++++++++++++++++++++ 2 files changed, 106 insertions(+), 1 deletion(-) diff --git a/core/state/pevm_statedb.go b/core/state/pevm_statedb.go index b4a1d8b01a..3930ffc646 100644 --- a/core/state/pevm_statedb.go +++ b/core/state/pevm_statedb.go @@ -263,7 +263,7 @@ func (pst *UncommittedDB) Selfdestruct6780(addr common.Address) { return } if obj.created { - pst.cache.selfDestruct(addr) + pst.SelfDestruct(addr) } } diff --git a/core/state/pevm_statedb_test.go b/core/state/pevm_statedb_test.go index ebddc22422..18559e4440 100644 --- a/core/state/pevm_statedb_test.go +++ b/core/state/pevm_statedb_test.go @@ -799,6 +799,111 @@ func TestPevmSelfDestruct(t *testing.T) { } } +func TestPevmSelfDestruct6780AndRevert(t *testing.T) { + shadow := newStateDB() + uncommitted := newUncommittedDB(newStateDB()) + //prepare: create an account, set state, set balance + // A1{key1: val1, balance: 100, accesslist: 0x33} + prepare := Tx{ + {"Create", Address1}, + {"SetState", Address1, "key1", "val1"}, + {"AddBalance", Address1, big.NewInt(100)}, + {"AddSlots", Address1, common.Hash{0x33}}, + } + prepareCheck := Checks{ + {"balance", Address1, big.NewInt(100)}, + {"state", Address1, "key1", "val1"}, + {"slot", Address1, common.Hash{0x33}, true}, + } + check := func(verify Checks, shadow, uncommitted vm.StateDB) { + if err := verify.Verify(shadow); err != nil { + t.Fatalf("maindb verify failed, err=%s", err.Error()) + } + if err := verify.Verify(uncommitted); err != nil { + t.Fatalf("uncommitted verify failed, err=%s", err.Error()) + } + } + prepare.Call(shadow) + prepare.Call(uncommitted) + check(prepareCheck, shadow, uncommitted) + + // do the following operations, and then selfdestruct, and then revert + // 1. add balance 200, + // 2. set state key1: val2 + // 3. add slot 0x34 + // 4. selfdestruct + // 5. revert + case1 := Tx{ + {"AddBalance", Address1, big.NewInt(200)}, + {"SetState", Address1, "key1", "val2"}, + {"SetState", Address1, "key2", "val0"}, + {"AddSlots", Address1, common.Hash{0x34}}, + } + case1SelfDestruct := Tx{ + {"SelfDestruct6780", Address1}, + } + beforeSelfDestruct := Checks{ + {"balance", Address1, big.NewInt(300)}, + {"state", Address1, "key1", "val2"}, + {"state", Address1, "key2", "val0"}, + {"slot", Address1, common.Hash{0x34}, true}, + {"slot", Address1, common.Hash{0x33}, true}, + } + beforeRevert := Checks{ + {"balance", Address1, big.NewInt(0)}, + {"state", Address1, "key1", "val2"}, + {"state", Address1, "key2", "val0"}, + {"slot", Address1, common.Hash{0x34}, true}, + {"slot", Address1, common.Hash{0x33}, true}, + } + afterRevert := Checks{ + {"balance", Address1, big.NewInt(100)}, + {"state", Address1, "key1", "val1"}, + {"state", Address1, "key2", ""}, + {"slot", Address1, common.Hash{0x34}, false}, + {"slot", Address1, common.Hash{0x33}, true}, + } + // run the case1 on shadow + snapshot := shadow.Snapshot() + case1.Call(shadow) + if err := beforeSelfDestruct.Verify(shadow); err != nil { + t.Fatalf("maindb ut failed, err:%s", err.Error()) + } + case1SelfDestruct.Call(shadow) + if err := beforeRevert.Verify(shadow); err != nil { + t.Fatalf("maindb ut failed, err:%s", err.Error()) + } + shadow.RevertToSnapshot(snapshot) + if err := afterRevert.Verify(shadow); err != nil { + t.Fatalf("maindb ut failed, err:%s", err.Error()) + } + + // now on uncommitted + snapshot = uncommitted.Snapshot() + case1.Call(uncommitted) + if err := beforeSelfDestruct.Verify(uncommitted); err != nil { + t.Fatalf("uncommitted ut failed, err:%s", err.Error()) + } + case1SelfDestruct.Call(uncommitted) + if err := beforeRevert.Verify(uncommitted); err != nil { + t.Fatalf("uncommitted ut failed, err:%s", err.Error()) + } + uncommitted.RevertToSnapshot(snapshot) + if err := afterRevert.Verify(uncommitted); err != nil { + t.Fatalf("uncommitted ut failed, err:%s", err.Error()) + } + + // now compare the mercle root of shadow and uncommitted + shadow.Finalise(true) + if err := uncommitted.Merge(true); err != nil { + t.Fatalf("ut failed, err:%s", err.Error()) + } + uncommitted.Finalise(true) + if shadow.IntermediateRoot(true) != uncommitted.maindb.IntermediateRoot(true) { + t.Fatalf("ut failed, err: the mercle root of shadow and uncommitted are different") + } +} + func TestPevmSelfDestructAndRevert(t *testing.T) { shadow := newStateDB() uncommitted := newUncommittedDB(newStateDB()) From 938ac30b35608b50de2fcb49388ff3b404fc2789 Mon Sep 17 00:00:00 2001 From: andyzhang2023 Date: Fri, 25 Oct 2024 09:18:29 +0800 Subject: [PATCH 4/7] UncommittedDB of PEVM shutdown when an invalid snapshot id found (just like the StateDB do) --- core/state/pevm_journal.go | 3 --- core/state/pevm_statedb.go | 4 +++- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/core/state/pevm_journal.go b/core/state/pevm_journal.go index a84349b94e..b9df4e1eca 100644 --- a/core/state/pevm_journal.go +++ b/core/state/pevm_journal.go @@ -292,9 +292,6 @@ func (j *ujournal) append(st ustate) { } func (j *ujournal) revertTo(db *UncommittedDB, snapshot int) { - if snapshot < 0 || snapshot >= len(*j) { - return // invalid snapshot index - } for i := len(*j) - 1; i >= snapshot; i-- { (*j)[i].revert(db) } diff --git a/core/state/pevm_statedb.go b/core/state/pevm_statedb.go index 3930ffc646..14f5224753 100644 --- a/core/state/pevm_statedb.go +++ b/core/state/pevm_statedb.go @@ -349,8 +349,10 @@ func (pst *UncommittedDB) AddSlotToAccessList(addr common.Address, slot common.H // =============================================== // Snapshot Methods -// (is it necessary to do snapshot and revert ?) func (pst *UncommittedDB) RevertToSnapshot(id int) { + if id < 0 || id > len(pst.journal) { + panic(fmt.Sprintf("invalid snapshot index, out of range, snapshot:%d, len:%d", id, len(pst.journal))) + } pst.journal.revertTo(pst, id) } func (pst *UncommittedDB) Snapshot() int { From 8fbb333198ec54d135ee8f300da49da3422d4353 Mon Sep 17 00:00:00 2001 From: andyzhang2023 Date: Fri, 25 Oct 2024 15:21:42 +0800 Subject: [PATCH 5/7] fix: do initParallelRunner() only once no matter how many chains are created by NewBlockChain() --- core/parallel_state_scheduler.go | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/core/parallel_state_scheduler.go b/core/parallel_state_scheduler.go index 963a39ba88..4ad229c399 100644 --- a/core/parallel_state_scheduler.go +++ b/core/parallel_state_scheduler.go @@ -12,19 +12,22 @@ import ( ) var runner chan func() +var runnerOnce sync.Once func initParallelRunner(targetNum int) { - if targetNum == 0 { - targetNum = runtime.GOMAXPROCS(0) - } - runner = make(chan func(), targetNum) - for i := 0; i < targetNum; i++ { - go func() { - for f := range runner { - f() - } - }() - } + runnerOnce.Do(func() { + if targetNum == 0 { + targetNum = runtime.GOMAXPROCS(0) + } + runner = make(chan func(), targetNum) + for i := 0; i < targetNum; i++ { + go func() { + for f := range runner { + f() + } + }() + } + }) } func ParallelNum() int { From 3fd45f3c8d1d526a75f0755e4f68741c327527a6 Mon Sep 17 00:00:00 2001 From: andyzhang2023 Date: Fri, 25 Oct 2024 16:42:13 +0800 Subject: [PATCH 6/7] fixut: init the runner before running the cases of PevmProcessor, otherwise it will stuck forever because of a nil runner --- core/parallel_state_scheduler_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/core/parallel_state_scheduler_test.go b/core/parallel_state_scheduler_test.go index 6c181e532c..6908e00fa8 100644 --- a/core/parallel_state_scheduler_test.go +++ b/core/parallel_state_scheduler_test.go @@ -7,6 +7,7 @@ import ( "io/ioutil" "math/big" "os" + "runtime" "sync" "sync/atomic" "testing" @@ -81,6 +82,10 @@ func (mt *mockTx) To() int { return int(mt.req.value) } +func init() { + initParallelRunner(runtime.GOMAXPROCS(0)) +} + func (mt *mockTx) execute(req *PEVMTxRequest) *PEVMTxResult { result := &PEVMTxResult{ txReq: req, From 5ae6ffd5c7afa19d40cda592fe766a3703de4390 Mon Sep 17 00:00:00 2001 From: andyzhang2023 Date: Fri, 25 Oct 2024 16:52:52 +0800 Subject: [PATCH 7/7] fixut: err should be returned by a seprated method ConflictsToMaindb() when running conflict check cases --- core/state/pevm_statedb_test.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/core/state/pevm_statedb_test.go b/core/state/pevm_statedb_test.go index 18559e4440..d4443c8581 100644 --- a/core/state/pevm_statedb_test.go +++ b/core/state/pevm_statedb_test.go @@ -2215,10 +2215,13 @@ func runConflictCase(prepare, txs1, txs2 Txs, checks []Check) error { if err := txs2.Call(un2); err != nil { return fmt.Errorf("failed to call txs2, err:%s", err.Error()) } + if err := un1.ConflictsToMaindb(); err != nil { + return fmt.Errorf("failed to check conflicts of un1, err:%s", err.Error()) + } if err := un1.Merge(true); err != nil { return fmt.Errorf("failed to merge un1, err:%s", err.Error()) } - if err := un2.Merge(true); err == nil { + if err := un2.ConflictsToMaindb(); err == nil { return fmt.Errorf("un2 merge is expected to be failed") } for _, c := range checks {