-
Notifications
You must be signed in to change notification settings - Fork 321
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
enable transient storage feature #4204
Conversation
@@ -446,6 +448,8 @@ func getChainConfig(g genesis.Blockchain, height uint64, id uint32, getBlockTime | |||
} | |||
sumatraTimestamp := (uint64)(sumatraTime.Unix()) | |||
chainConfig.ShanghaiTime = &sumatraTimestamp | |||
//todo: enable Cancun | |||
chainConfig.CancunTime = &sumatraTimestamp |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need Hardfork cancun time, enable it for test here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use ToBeEnabledHeight
|
||
// Selfdestruct6780 implements EIP-6780 | ||
func (stateDB *StateDBAdapter) Selfdestruct6780(evmAddr common.Address) { | ||
//Todo: implement EIP-6780 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
implemented another PR
"expectedStatus": 1, | ||
"hasReturnValue": true, | ||
"rawReturnValue": "", | ||
"rawReturnValue": "0000000000000000000000000000000000000000000000000000000000000014", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"selfdestruct" has been deprecated. Note that, starting from the Cancun hard fork, the underlying opcode no longer deletes the code and data associated with an account and only transfers its Ether to the beneficiary
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is test for Shanghai EVM, your change should not break the test
|
@@ -11,7 +11,7 @@ GOVET=$(GOCMD) vet | |||
GOBUILD=$(GOCMD) build | |||
GOINSTALL=$(GOCMD) install | |||
GOCLEAN=$(GOCMD) clean | |||
GOTEST=$(GOCMD) test | |||
GOTEST=GOARCH=amd64 CGO_ENABLED=1 $(GOCMD) test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix gomonkey error in mac m1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we fix the test instead of here? this would limit the testing env to amd64 only
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4204 +/- ##
==========================================
+ Coverage 75.38% 76.78% +1.40%
==========================================
Files 303 340 +37
Lines 25923 29280 +3357
==========================================
+ Hits 19541 22483 +2942
- Misses 5360 5691 +331
- Partials 1022 1106 +84 ☔ View full report in Codecov by Sentry. |
@@ -25,7 +25,7 @@ jobs: | |||
- name: Set up Go | |||
uses: actions/setup-go@v3 | |||
with: | |||
go-version: 1.19.12 | |||
go-version: 1.21.4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this a must? if we want to upgrade go version, should do in another PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, go-ethereum has been upgraded to go1.21, we must upgrade too
@@ -225,10 +226,11 @@ func securityDeposit(ps *Params, stateDB vm.StateDB, gasLimit uint64) error { | |||
return action.ErrGasLimit | |||
} | |||
gasConsumed := new(big.Int).Mul(new(big.Int).SetUint64(ps.gas), ps.txCtx.GasPrice) | |||
if stateDB.GetBalance(ps.txCtx.Origin).Cmp(gasConsumed) < 0 { | |||
gasConsumedUint256 := uint256.NewInt(gasConsumed.Uint64()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gasConsumed := uint256.MustFromBig(new(big.Int).Mul(new(big.Int).SetUint64(ps.gas), ps.txCtx.GasPrice))
as above L203
Uint64() may overflow right?
accessListSnapshot map[int]*accessList | ||
// Transient storage | ||
transientStorage transientStorage | ||
transientStorageSnapshot map[int]transientStorage |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this PR has lots of change, and focus on making interface work with Cancun.
let's enable transient storage in another PR
@@ -234,8 +240,8 @@ func (stateDB *StateDBAdapter) CreateAccount(evmAddr common.Address) { | |||
} | |||
|
|||
// SubBalance subtracts balance from account | |||
func (stateDB *StateDBAdapter) SubBalance(evmAddr common.Address, amount *big.Int) { | |||
if amount.Cmp(big.NewInt(int64(0))) == 0 { | |||
func (stateDB *StateDBAdapter) SubBalance(evmAddr common.Address, amount *uint256.Int) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SubBalance(evmAddr common.Address, a256 *uint256.Int) {
amount := a256.ToBig()
then no need to change inside the func
@@ -263,9 +269,9 @@ func (stateDB *StateDBAdapter) SubBalance(evmAddr common.Address, amount *big.In | |||
} | |||
|
|||
// AddBalance adds balance to account | |||
func (stateDB *StateDBAdapter) AddBalance(evmAddr common.Address, amount *big.Int) { | |||
func (stateDB *StateDBAdapter) AddBalance(evmAddr common.Address, amount *uint256.Int) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above
} | ||
// clears the account balance | ||
actBalance := new(big.Int).Set(s.Balance) | ||
actBalance := new(uint256.Int).SetBytes(s.Balance.Bytes()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we don't need to change this?
@@ -447,13 +453,13 @@ func (stateDB *StateDBAdapter) Suicide(evmAddr common.Address) bool { | |||
// before calling Suicide, EVM will transfer the contract's balance to beneficiary | |||
// need to create a transaction log on successful suicide | |||
if stateDB.lastAddBalanceAmount.Cmp(actBalance) == 0 { | |||
if stateDB.lastAddBalanceAmount.Cmp(big.NewInt(0)) > 0 { | |||
if stateDB.lastAddBalanceAmount.Cmp(common.U2560) > 0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need to change?
from, _ := address.FromBytes(evmAddr[:]) | ||
stateDB.addTransactionLogs(&action.TransactionLog{ | ||
Type: iotextypes.TransactionLogType_IN_CONTRACT_TRANSFER, | ||
Sender: from.String(), | ||
Recipient: stateDB.lastAddBalanceAddr, | ||
Amount: stateDB.lastAddBalanceAmount, | ||
Amount: stateDB.lastAddBalanceAmount.ToBig(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need to change?
addrHash := hash.BytesToHash160(evmAddr.Bytes()) | ||
_, ok := stateDB.suicided[addrHash] | ||
return ok | ||
} | ||
|
||
// 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.GetTransientState(addr, key) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's do in next PR
@@ -522,6 +543,8 @@ func (stateDB *StateDBAdapter) Prepare(rules params.Rules, sender, coinbase comm | |||
if !rules.IsBerlin { | |||
return | |||
} | |||
// Clear out any leftover from previous executions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in next PR
@@ -539,6 +562,9 @@ 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in next PR
break | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in next PR
@@ -933,3 +933,45 @@ func TestSortMap(t *testing.T) { | |||
require.True(testFunc(t, sm)) | |||
}) | |||
} | |||
|
|||
func TestStateDBTransientStorage(t *testing.T) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in next PR
@@ -986,8 +986,8 @@ func TestContractStakingIndexerVotes(t *testing.T) { | |||
d2Bts, err := indexer.BucketsByCandidate(delegate2, height) | |||
r.NoError(err) | |||
r.Len(d2Bts, 2) | |||
slices.SortFunc(d2Bts, func(i, j *staking.VoteBucket) bool { | |||
return i.Index < j.Index | |||
slices.SortFunc(d2Bts, func(i, j *staking.VoteBucket) int { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why need to change these?
@@ -2140,8 +2140,8 @@ func stake(lsdABI abi.ABI, bc blockchain.Blockchain, sf factory.Factory, dao blo | |||
r.EqualValues(iotextypes.ReceiptStatus_Success, receipts[0].Status) | |||
buckets, err := indexer.Buckets(blk.Height()) | |||
r.NoError(err) | |||
slices.SortFunc(buckets, func(i, j *contractstaking.Bucket) bool { | |||
return i.Index < j.Index | |||
slices.SortFunc(buckets, func(i, j *contractstaking.Bucket) int { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why need to change these?
@@ -347,8 +347,8 @@ func TestConstantinople(t *testing.T) { | |||
require.Equal(isSumatra, chainRules.IsShanghai) | |||
|
|||
// Cancun, Prague not yet enabled | |||
require.False(evmChainConfig.IsCancun(evm.Context.Time)) | |||
require.False(evmChainConfig.IsPrague(evm.Context.Time)) | |||
require.False(evmChainConfig.IsCancun(big.NewInt(0), evm.Context.Time)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is the first input?
Description
EVM upgrade to v1.13.12
enable transient storage feature
add tests
Fixes #4177
Type of change
Please delete options that are not relevant.
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
Test Configuration:
Checklist: