Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[evm] EIP-1153 enable transient storage feature #4214

Merged
merged 19 commits into from
Apr 12, 2024
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions action/protocol/execution/evm/evm.go
Original file line number Diff line number Diff line change
Expand Up @@ -447,6 +447,12 @@ func getChainConfig(g genesis.Blockchain, height uint64, id uint32, getBlockTime
}
sumatraTimestamp := (uint64)(sumatraTime.Unix())
chainConfig.ShanghaiTime = &sumatraTimestamp
upernavikTime, err := getBlockTime(g.UpernavikBlockHeight)
if err != nil {
return nil, err
}
upernavikTimestamp := (uint64)(upernavikTime.Unix())
chainConfig.CancunTime = &upernavikTimestamp
return &chainConfig, nil
}

Expand Down
6 changes: 4 additions & 2 deletions action/protocol/execution/evm/evm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -355,8 +355,10 @@ func TestConstantinople(t *testing.T) {
require.Equal(isSumatra, chainRules.IsMerge)
require.Equal(isSumatra, chainRules.IsShanghai)

// Cancun, Prague not yet enabled
require.False(evmChainConfig.IsCancun(big.NewInt(int64(e.height)), evm.Context.Time))
// Upernavik = enable Cancun
isUpernavik := g.IsUpernavik(e.height)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comments need updates

require.Equal(isUpernavik, chainRules.IsCancun)
//Prague not yet enabled
require.False(evmChainConfig.IsPrague(big.NewInt(int64(e.height)), evm.Context.Time))

// test basefee
Expand Down
104 changes: 66 additions & 38 deletions action/protocol/execution/evm/evmstatedbadapter.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,24 +43,27 @@

// StateDBAdapter represents the state db adapter for evm to access iotx blockchain
StateDBAdapter struct {
sm protocol.StateManager
logs []*action.Log
transactionLogs []*action.TransactionLog
err error
blockHeight uint64
executionHash hash.Hash256
lastAddBalanceAddr string
lastAddBalanceAmount *big.Int
refund uint64
refundSnapshot map[int]uint64
cachedContract contractMap
contractSnapshot map[int]contractMap // snapshots of contracts
selfDestructed deleteAccount // account/contract calling SelfDestruct
selfDestructedSnapshot map[int]deleteAccount // snapshots of SelfDestruct accounts
preimages preimageMap
preimageSnapshot map[int]preimageMap
accessList *accessList // per-transaction access list
accessListSnapshot map[int]*accessList
sm protocol.StateManager
logs []*action.Log
transactionLogs []*action.TransactionLog
err error
blockHeight uint64
executionHash hash.Hash256
lastAddBalanceAddr string
lastAddBalanceAmount *big.Int
refund uint64
refundSnapshot map[int]uint64
cachedContract contractMap
contractSnapshot map[int]contractMap // snapshots of contracts
selfDestructed deleteAccount // account/contract calling SelfDestruct
selfDestructedSnapshot map[int]deleteAccount // snapshots of SelfDestruct accounts
preimages preimageMap
preimageSnapshot map[int]preimageMap
accessList *accessList // per-transaction access list
accessListSnapshot map[int]*accessList
// Transient storage
transientStorage transientStorage
transientStorageSnapshot map[int]transientStorage
logsSnapshot map[int]int // logs is an array, save len(logs) at time of snapshot suffices
txLogsSnapshot map[int]int
notFixTopicCopyBug bool
Expand Down Expand Up @@ -170,23 +173,25 @@
opts ...StateDBAdapterOption,
) (*StateDBAdapter, error) {
s := &StateDBAdapter{
sm: sm,
logs: []*action.Log{},
err: nil,
blockHeight: blockHeight,
executionHash: executionHash,
lastAddBalanceAmount: new(big.Int),
refundSnapshot: make(map[int]uint64),
cachedContract: make(contractMap),
contractSnapshot: make(map[int]contractMap),
selfDestructed: make(deleteAccount),
selfDestructedSnapshot: make(map[int]deleteAccount),
preimages: make(preimageMap),
preimageSnapshot: make(map[int]preimageMap),
accessList: newAccessList(),
accessListSnapshot: make(map[int]*accessList),
logsSnapshot: make(map[int]int),
txLogsSnapshot: make(map[int]int),
sm: sm,
logs: []*action.Log{},
err: nil,
blockHeight: blockHeight,
executionHash: executionHash,
lastAddBalanceAmount: new(big.Int),
refundSnapshot: make(map[int]uint64),
cachedContract: make(contractMap),
contractSnapshot: make(map[int]contractMap),
selfDestructed: make(deleteAccount),
selfDestructedSnapshot: make(map[int]deleteAccount),
preimages: make(preimageMap),
preimageSnapshot: make(map[int]preimageMap),
accessList: newAccessList(),
accessListSnapshot: make(map[int]*accessList),
transientStorage: newTransientStorage(),
transientStorageSnapshot: make(map[int]transientStorage),
logsSnapshot: make(map[int]int),
txLogsSnapshot: make(map[int]int),
}
for _, opt := range opts {
if err := opt(s); err != nil {
Expand Down Expand Up @@ -478,13 +483,16 @@

// SetTransientState sets transient storage for a given account
func (stateDB *StateDBAdapter) SetTransientState(addr common.Address, key, value common.Hash) {
log.S().Panic("SetTransientState not implemented")
prev := stateDB.transientStorage.Get(addr, key)
if prev == value {
return

Check warning on line 488 in action/protocol/execution/evm/evmstatedbadapter.go

View check run for this annotation

Codecov / codecov/patch

action/protocol/execution/evm/evmstatedbadapter.go#L488

Added line #L488 was not covered by tests
}
stateDB.transientStorage.Set(addr, key, value)
}

// GetTransientState gets transient storage for a given account.
func (stateDB *StateDBAdapter) GetTransientState(addr common.Address, key common.Hash) common.Hash {
log.S().Panic("GetTransientState not implemented")
return common.Hash{}
return stateDB.transientStorage.Get(addr, key)
}

// Selfdestruct6780 implements EIP-6780
Expand Down Expand Up @@ -530,6 +538,8 @@
if !rules.IsBerlin {
return
}
// Clear out any leftover from previous executions
stateDB.accessList = newAccessList()

Check warning on line 542 in action/protocol/execution/evm/evmstatedbadapter.go

View check run for this annotation

Codecov / codecov/patch

action/protocol/execution/evm/evmstatedbadapter.go#L542

Added line #L542 was not covered by tests
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is not necessary? we are creating a new StateDBAdpator for each tx

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for safety, it's best to add

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the relationship with eip-1153?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for safety, it's best to add

for what kind of safety? We need to be clear what we want to achieve.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stateDB.AddAddressToAccessList(sender)
if dst != nil {
stateDB.AddAddressToAccessList(*dst)
Expand All @@ -547,6 +557,8 @@
if rules.IsShanghai { // EIP-3651: warm coinbase
stateDB.AddAddressToAccessList(coinbase)
}
// Reset transient storage at the beginning of transaction execution
stateDB.transientStorage = newTransientStorage()

Check warning on line 561 in action/protocol/execution/evm/evmstatedbadapter.go

View check run for this annotation

Codecov / codecov/patch

action/protocol/execution/evm/evmstatedbadapter.go#L561

Added line #L561 was not covered by tests
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same thing, not necessary

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is best to add

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as discused, let's remove this, it is done in NewStateDBAdapter

}

// AddressInAccessList returns true if the given address is in the access list
Expand Down Expand Up @@ -623,6 +635,18 @@
}
}
}
//restore transientStorage
stateDB.transientStorage = stateDB.transientStorageSnapshot[snapshot]
{
delete(stateDB.transientStorageSnapshot, snapshot)
for i := snapshot + 1; ; i++ {
if _, ok := stateDB.transientStorageSnapshot[i]; ok {
delete(stateDB.transientStorageSnapshot, i)
} else {
break
}
}
}
// restore logs and txLogs
if stateDB.revertLog {
stateDB.logs = stateDB.logs[:stateDB.logsSnapshot[snapshot]]
Expand Down Expand Up @@ -750,6 +774,8 @@
stateDB.preimageSnapshot[sn] = p
// save a copy of access list
stateDB.accessListSnapshot[sn] = stateDB.accessList.Copy()
// save a copy of transient storage
stateDB.transientStorageSnapshot[sn] = stateDB.transientStorage.Copy()
millken marked this conversation as resolved.
Show resolved Hide resolved
return sn
}

Expand Down Expand Up @@ -1089,6 +1115,8 @@
stateDB.preimageSnapshot = make(map[int]preimageMap)
stateDB.accessList = newAccessList()
stateDB.accessListSnapshot = make(map[int]*accessList)
stateDB.transientStorage = newTransientStorage()
stateDB.transientStorageSnapshot = make(map[int]transientStorage)
stateDB.logsSnapshot = make(map[int]int)
stateDB.txLogsSnapshot = make(map[int]int)
stateDB.logs = []*action.Log{}
Expand Down
94 changes: 86 additions & 8 deletions action/protocol/execution/evm/evmstatedbadapter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -342,10 +342,13 @@ var tests = []stateDBTest{
[]access{
{_c1, []common.Hash{_k1, _k2}, []common.Hash{_k3, _k4}, false},
},
[]transient{
{_c1, _k1, _v1},
},
[]*types.Log{
newTestLog(_c3), newTestLog(_c2), newTestLog(_c1),
},
3, 0,
3, 0, 1,
"io1q87zge3ngux0v2hz49tdy85dfqwr560pj9mk7r", "io1q87zge3ngux0v2hz49tdy85dfqwr560pj9mk7r", "",
},
{
Expand Down Expand Up @@ -373,10 +376,13 @@ var tests = []stateDBTest{
{_c1, []common.Hash{_k3, _k4}, nil, true},
{_c2, []common.Hash{_k1, _k3}, []common.Hash{_k2, _k4}, false},
},
[]transient{
{_c2, _k2, _v2},
},
[]*types.Log{
newTestLog(_c4),
},
4, 1,
4, 1, 2,
"io1zg0qrlpyvc68pnmz4c4f2mfc6jqu8f57jjy09q",
"io1j4kjr6x5s8p6dyqlcfrxxdrsea32u2hpvpl5us",
"io1x3cv7c4w922k6wx5s8p6d8sjrcqlcfrxhkn5xe",
Expand All @@ -398,10 +404,13 @@ var tests = []stateDBTest{
[]access{
{_c2, []common.Hash{_k2, _k4}, nil, true},
},
[]transient{
{_c3, _k3, _v3},
},
[]*types.Log{
newTestLog(_c1), newTestLog(_c2),
},
6, 2,
6, 2, 3,
"io1x3cv7c4w922k6wx5s8p6d8sjrcqlcfrxhkn5xe",
"io1q2hz49tdy85dfqwr560pge3ngux0vf0vmhanad",
"io1q87zge3ngux0v2hz49tdy85dfqwr560pj9mk7r",
Expand Down Expand Up @@ -481,13 +490,17 @@ func TestSnapshotRevertAndCommit(t *testing.T) {
require.False(sOk)
}
}
//set transient storage
for _, e := range test.transient {
stateDB.SetTransientState(e.addr, e.k, e.v)
}
// set logs and txLogs
for _, l := range test.logs {
stateDB.AddLog(l)
}

require.Equal(test.logSize, len(stateDB.logs))
require.Equal(test.txLogSize, len(stateDB.transactionLogs))
require.Equal(test.transientSize, len(stateDB.transientStorage))
require.Equal(test.logAddr, stateDB.logs[test.logSize-1].Address)
if test.txLogSize > 0 {
require.Equal(test.txSender, stateDB.transactionLogs[test.txLogSize-1].Sender)
Expand Down Expand Up @@ -526,8 +539,11 @@ func TestSnapshotRevertAndCommit(t *testing.T) {
{_c1, []common.Hash{_k1, _k2, _k3, _k4}, nil, true},
{_c2, []common.Hash{_k1, _k2, _k3, _k4}, nil, true},
},
[]transient{
{_c3, _k3, _v3},
},
nil,
6, 2,
6, 2, 3,
"io1x3cv7c4w922k6wx5s8p6d8sjrcqlcfrxhkn5xe",
"io1q2hz49tdy85dfqwr560pge3ngux0vf0vmhanad",
"io1q87zge3ngux0v2hz49tdy85dfqwr560pj9mk7r",
Expand Down Expand Up @@ -556,8 +572,11 @@ func TestSnapshotRevertAndCommit(t *testing.T) {
{_c1, []common.Hash{_k1, _k2, _k3, _k4}, nil, true},
{_c2, []common.Hash{_k1, _k3}, []common.Hash{_k2, _k4}, true},
},
[]transient{
{_c2, _k2, _v2},
},
nil,
4, 1,
4, 1, 2,
"io1zg0qrlpyvc68pnmz4c4f2mfc6jqu8f57jjy09q",
"io1j4kjr6x5s8p6dyqlcfrxxdrsea32u2hpvpl5us",
"io1x3cv7c4w922k6wx5s8p6d8sjrcqlcfrxhkn5xe",
Expand Down Expand Up @@ -590,8 +609,11 @@ func TestSnapshotRevertAndCommit(t *testing.T) {
{_c1, []common.Hash{_k1, _k2}, []common.Hash{_k3, _k4}, true},
{_c2, nil, []common.Hash{_k1, _k2, _k3, _k4}, false},
},
[]transient{
{_c1, _k1, _v1},
},
nil,
3, 0,
3, 0, 1,
"io1q87zge3ngux0v2hz49tdy85dfqwr560pj9mk7r",
"io1q87zge3ngux0v2hz49tdy85dfqwr560pj9mk7r",
"",
Expand Down Expand Up @@ -638,6 +660,10 @@ func TestSnapshotRevertAndCommit(t *testing.T) {
require.False(sOk)
}
}
//test transient storage
for _, e := range test.transient {
require.Equal(e.v, stateDB.GetTransientState(e.addr, e.k))
}
}
// test SelfDestruct/exist
for _, e := range test.selfDestruct {
Expand Down Expand Up @@ -669,14 +695,16 @@ func TestSnapshotRevertAndCommit(t *testing.T) {
require.Equal(1, len(stateDB.selfDestructedSnapshot))
require.Equal(1, len(stateDB.preimageSnapshot))
require.Equal(1, len(stateDB.accessListSnapshot))
require.Equal(1, len(stateDB.transientStorageSnapshot))
require.Equal(1, len(stateDB.refundSnapshot))
} else {
require.Equal(3, len(stateDB.contractSnapshot))
require.Equal(3, len(stateDB.selfDestructedSnapshot))
require.Equal(3, len(stateDB.preimageSnapshot))
// refund fix and accessList are introduced after fixSnapshot
// refund fix and accessList、transient storage are introduced after fixSnapshot
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems a chinese version comma

// so their snapshot are always properly cleared
require.Zero(len(stateDB.accessListSnapshot))
require.Zero(len(stateDB.transientStorageSnapshot))
require.Zero(len(stateDB.refundSnapshot))
}
// commit snapshot 0's state
Expand Down Expand Up @@ -934,3 +962,53 @@ func TestSortMap(t *testing.T) {
require.True(testFunc(t, sm))
})
}

func TestStateDBTransientStorage(t *testing.T) {
require := require.New(t)
ctrl := gomock.NewController(t)
sm, err := initMockStateManager(ctrl)
require.NoError(err)
var opts []StateDBAdapterOption
opts = append(opts,
NotFixTopicCopyBugOption(),
FixSnapshotOrderOption(),
)
state, err := NewStateDBAdapter(sm, 1, hash.ZeroHash256, opts...)
if err != nil {
t.Fatal(err)
}
var (
addr0 = common.Address{}
addr1 = common.HexToAddress("1234567890")
k1 = common.Hash{}
k2 = common.HexToHash("34567890ab")
v1 = common.HexToHash("567890abcd")
v2 = common.HexToHash("7890abcdef")
)
tests := []struct {
addr common.Address
key, val common.Hash
}{
{addr0, k1, v1},
{addr0, k2, v2},
{addr1, k1, v2},
{addr1, k2, v1},
}
for _, test := range tests {
addr := test.addr
key := test.key
value := test.val
sn := state.Snapshot()
state.SetTransientState(addr, key, value)
require.Equal(value, state.GetTransientState(addr, key))

// revert the transient state being set and then check that the
// value is now the empty hash
state.RevertToSnapshot(sn)
require.Equal(common.Hash{}, state.GetTransientState(addr, key))

state.SetTransientState(addr, key, value)
require.Equal(value, state.GetTransientState(addr, key))
}

}
Loading
Loading