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

millken · 2024-03-28T08:47:34Z

Description

Implemented EIP-1153

Type of change

Please delete options that are not relevant.

[] Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
[] Code refactor or improvement
[] Breaking change (fix or feature that would cause a new or changed behavior of existing functionality)
[] This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

make test
[] fullsync
[] Other test (please specify)

Test Configuration:

Firmware version:
Hardware:
Toolchain:
SDK:

Checklist:

[] My code follows the style guidelines of this project
[] I have performed a self-review of my code
[] I have commented my code, particularly in hard-to-understand areas
[] I have made corresponding changes to the documentation
[] My changes generate no new warnings
[] I have added tests that prove my fix is effective or that my feature works
[] New and existing unit tests pass locally with my changes
[] Any dependent changes have been merged and published in downstream modules

millken · 2024-03-29T03:20:36Z

action/protocol/execution/evm/evm_test.go

@@ -259,7 +259,7 @@ func TestConstantinople(t *testing.T) {
 		},
 		{
 			"io1pcg2ja9krrhujpazswgz77ss46xgt88afqlk6y",
-			1261440000, // = 200*365*24*3600/5, around 200 years later
+			39275560,


it must less genesis default UpernavikBlockHeight

check past PR, you need to break it to 2 parts

after Tsunami - Upernavik { } after Upernavik { }

still not fixed?

millken · 2024-03-29T03:26:49Z

action/protocol/execution/testdata-cancun/transientstorage.sol

+    function tload(uint location) public view returns (uint value) {
+        assembly {
+            value := tload(location)
+        }


Transient storage is accessible to smart contracts via 2 new opcodes, TLOAD and TSTORE

refer: https://eips.ethereum.org/EIPS/eip-1153

codecov · 2024-03-29T03:50:34Z

Codecov Report

Attention: Patch coverage is 92.30769% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 76.75%. Comparing base (d716bb9) to head (46adced).
Report is 22 commits behind head on master.

❗ Current head 46adced differs from pull request most recent head 4b76ef0. Consider uploading reports for the commit 4b76ef0 to get more accurate results

Files	Patch %	Lines
action/protocol/execution/evm/evmstatedbadapter.go	91.42%	2 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4214      +/-   ##
==========================================
+ Coverage   76.51%   76.75%   +0.24%     
==========================================
  Files         340      340              
  Lines       29273    29314      +41     
==========================================
+ Hits        22397    22501     +104     
+ Misses       5761     5707      -54     
+ Partials     1115     1106       -9

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

action/protocol/execution/evm/evmstatedbadapter.go

dustinxie · 2024-03-31T22:04:33Z

action/protocol/execution/evm/evmstatedbadapter.go

@@ -530,6 +544,8 @@ func (stateDB *StateDBAdapter) Prepare(rules params.Rules, sender, coinbase comm
 	if !rules.IsBerlin {
 		return
 	}
+	// Clear out any leftover from previous executions
+	stateDB.accessList = newAccessList()


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

for safety, it's best to add

What is the relationship with eip-1153?

for safety, it's best to add

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

the code has updated in
https://github.com/ethereum/go-ethereum/pull/26003/files#diff-c3757dc9e9d868f63bc84a0cc67159c1d5c22cc5d8c9468757098f0492e0658cR1073-R1074

i think we missed it

dustinxie · 2024-03-31T22:05:18Z

action/protocol/execution/evm/evmstatedbadapter.go

@@ -547,6 +563,8 @@ func (stateDB *StateDBAdapter) Prepare(rules params.Rules, sender, coinbase comm
 	if rules.IsShanghai { // EIP-3651: warm coinbase
 		stateDB.AddAddressToAccessList(coinbase)
 	}
+	// Reset transient storage at the beginning of transaction execution
+	stateDB.transientStorage = newTransientStorage()


same thing, not necessary

it is best to add

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

action/protocol/execution/evm/evmstatedbadapter.go

action/protocol/execution/evm/evmstatedbadapter_test.go

action/protocol/execution/testdata-cancun/transientstorage.sol

action/protocol/execution/evm/evmstatedbadapter.go

envestcc · 2024-04-01T14:16:10Z

action/protocol/execution/evm/evm_test.go

@@ -356,7 +356,8 @@ func TestConstantinople(t *testing.T) {
 		require.Equal(isSumatra, chainRules.IsShanghai)

 		// Cancun, Prague not yet enabled
-		require.False(evmChainConfig.IsCancun(big.NewInt(int64(e.height)), evm.Context.Time))
+		isUpernavik := g.IsUpernavik(e.height)


comments need updates

dustinxie · 2024-04-02T22:34:13Z

action/protocol/execution/evm/evmstatedbadapter_test.go

+		sn := state.Snapshot()
+		state.SetTransientState(addr, key, value)
+		// the retrieved value should equal what was set
+		if got := state.GetTransientState(addr, key); got != value {


let's be consistent with testing code style, use require.Equal(value, got)

dustinxie · 2024-04-02T22:34:42Z

action/protocol/execution/evm/evmstatedbadapter_test.go

+		// value is now the empty hash
+		state.RevertToSnapshot(sn)
+		if got, exp := state.GetTransientState(addr, key), (common.Hash{}); exp != got {
+			t.Fatalf("transient storage mismatch: have %x, want %x", got, exp)


same here, use current testing code style

dustinxie · 2024-04-02T22:34:49Z

action/protocol/execution/evm/evmstatedbadapter_test.go

-		t.Fatalf("transient storage mismatch: have %x, want %x", got, value)
+		// reset transient state
+		state.SetTransientState(addr, key, value)
+		if got := state.GetTransientState(addr, key); got != value {


dustinxie · 2024-04-02T22:38:21Z

action/protocol/execution/evm/evmstatedbadapter_test.go

@@ -677,6 +704,7 @@ func TestSnapshotRevertAndCommit(t *testing.T) {
 			// refund fix and accessList are introduced after fixSnapshot


update comment, add transient storage

dustinxie · 2024-04-08T03:08:06Z

action/protocol/execution/evm/evmstatedbadapter_test.go

@@ -701,7 +701,7 @@ func TestSnapshotRevertAndCommit(t *testing.T) {
 			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


seems a chinese version comma

dustinxie · 2024-04-08T22:44:11Z

action/protocol/execution/testdata-cancun/transientstorage.json

+        "rawReturnValue": "0000000000000000000000000000000000000000000000000000000000000003",
+        "comment": "call getAddress"
+    }]
+}


add newline

dustinxie · 2024-04-09T05:15:50Z

action/protocol/execution/evm/evmstatedbadapter_test.go

@@ -701,7 +701,7 @@ func TestSnapshotRevertAndCommit(t *testing.T) {
 			require.Equal(3, len(stateDB.contractSnapshot))
 			require.Equal(3, len(stateDB.selfDestructedSnapshot))
 			require.Equal(3, len(stateDB.preimageSnapshot))
-			// refund fix and accessList、transient storage are introduced after fixSnapshot
+			// refund fix and accessList,transient storage are introduced after fixSnapshot


refund fix, accessList, and transient

dustinxie · 2024-04-09T22:48:52Z

action/protocol/execution/evm/evm_test.go

@@ -257,6 +257,16 @@ func TestConstantinople(t *testing.T) {
 			action.EmptyAddress,
 			29275561,
 		},
+		// after Tsunami - Upernavik
+		{
+			action.EmptyAddress,


should be "io1pcg2ja9krrhujpazswgz77ss46xgt88afqlk6y"

dustinxie · 2024-04-09T22:49:17Z

action/protocol/execution/evm/evm_test.go

@@ -257,6 +257,16 @@ func TestConstantinople(t *testing.T) {
 			action.EmptyAddress,
 			29275561,
 		},
+		// after Tsunami - Upernavik


and this comment line should be at L255

sonarcloud · 2024-04-11T06:14:58Z

Quality Gate failed

Failed conditions
9.8% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud

millken added 2 commits March 28, 2024 16:43

[evm] enable transient storage feature

4ab40f0

[evm] enable transient storage feature

16f63a0

millken requested review from CoderZhi, dustinxie, Liuhaai and envestcc as code owners March 28, 2024 08:47

millken marked this pull request as draft March 28, 2024 09:02

millken marked this pull request as ready for review March 29, 2024 02:18

millken added 3 commits March 29, 2024 10:18

Merge branch 'master' into issue4177-eip1153

75b545c

[evm] update cancuntime

47bf31c

fix tests

9bf2c28

millken commented Mar 29, 2024

View reviewed changes

dustinxie reviewed Mar 31, 2024

View reviewed changes

action/protocol/execution/evm/evmstatedbadapter.go Outdated Show resolved Hide resolved

dustinxie reviewed Mar 31, 2024

View reviewed changes

action/protocol/execution/evm/evmstatedbadapter.go Outdated Show resolved Hide resolved

dustinxie reviewed Mar 31, 2024

View reviewed changes

action/protocol/execution/evm/evmstatedbadapter.go Outdated Show resolved Hide resolved

dustinxie reviewed Mar 31, 2024

View reviewed changes

action/protocol/execution/evm/evmstatedbadapter.go Outdated Show resolved Hide resolved

dustinxie reviewed Mar 31, 2024

View reviewed changes

action/protocol/execution/evm/evmstatedbadapter_test.go Outdated Show resolved Hide resolved

dustinxie reviewed Mar 31, 2024

View reviewed changes

action/protocol/execution/testdata-cancun/transientstorage.sol Outdated Show resolved Hide resolved

dustinxie reviewed Mar 31, 2024

View reviewed changes

action/protocol/execution/evm/evmstatedbadapter.go Show resolved Hide resolved

millken added 3 commits April 1, 2024 11:19

Merge branch 'master' into issue4177-eip1153

cd7beb2

fix comments issue

64d1b5d

fix comments issue

4f12e20

envestcc reviewed Apr 1, 2024

View reviewed changes

dustinxie reviewed Apr 2, 2024

View reviewed changes

fix comments

5d00100

dustinxie reviewed Apr 8, 2024

View reviewed changes

millken added 2 commits April 9, 2024 08:34

Merge branch 'master' into issue4177-eip1153

095ca75

fix comments

214787c

dustinxie reviewed Apr 9, 2024

View reviewed changes

millken added 3 commits April 9, 2024 14:26

fix comments

3a19630

Merge branch 'master' into issue4177-eip1153

46adced

fix comments

77d80fd

dustinxie reviewed Apr 9, 2024

View reviewed changes

millken added 5 commits April 10, 2024 11:08

fix comments

5c25a48

fix comments

34b94d6

Merge branch 'master' into issue4177-eip1153

d5656e4

fix comments

03ac0dd

Merge branch 'master' into issue4177-eip1153

4b76ef0

dustinxie approved these changes Apr 11, 2024

View reviewed changes

envestcc approved these changes Apr 12, 2024

View reviewed changes

millken merged commit afbbbd2 into iotexproject:master Apr 12, 2024
2 of 3 checks passed

millken deleted the issue4177-eip1153 branch April 12, 2024 01:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

millken commented Mar 28, 2024 •

edited

Loading

millken Mar 29, 2024

dustinxie Mar 31, 2024

dustinxie Apr 8, 2024

millken Apr 9, 2024

millken Mar 29, 2024

codecov bot commented Mar 29, 2024 •

edited

Loading

dustinxie Mar 31, 2024

millken Apr 1, 2024

envestcc Apr 1, 2024

CoderZhi Apr 6, 2024

millken Apr 7, 2024

dustinxie Mar 31, 2024

millken Apr 1, 2024

dustinxie Apr 9, 2024

envestcc Apr 1, 2024

dustinxie Apr 2, 2024

dustinxie Apr 2, 2024

dustinxie Apr 2, 2024

dustinxie Apr 2, 2024

dustinxie Apr 8, 2024

dustinxie Apr 8, 2024

dustinxie Apr 9, 2024

dustinxie Apr 9, 2024

dustinxie Apr 9, 2024

sonarcloud bot commented Apr 11, 2024

		@@ -677,6 +704,7 @@ func TestSnapshotRevertAndCommit(t *testing.T) {
		// refund fix and accessList are introduced after fixSnapshot

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

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

Conversation

millken commented Mar 28, 2024 • edited Loading

Description

Type of change

How Has This Been Tested?

Checklist:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Mar 29, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarcloud bot commented Apr 11, 2024

Quality Gate failed

millken commented Mar 28, 2024 •

edited

Loading

codecov bot commented Mar 29, 2024 •

edited

Loading