[WIP] Enable blockchain EEST #223
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
test: Enable Shanghai and Cancun EIPs blockchain test by Cancun and Prague forks
test: Enable prague's EIP7623
* Enable eip 2935 on blockchain EEST * Fix cyclic import * Fix for lint
test: EETS Set precompile correctly
* Enable old forks other than frontier and byzantium * Restore all old fork tests
ulbqb
commented
Jan 22, 2025
Comment on lines
+315
to
+317
| if CreateContractWithCodeFormatInExecutionSpecTest { | ||
| stateDB.CreateSmartContractAccount(addr, params.CodeFormatEVM, g.Config.Rules(new(big.Int).SetUint64(g.Number))) | ||
| } |
There was a problem hiding this comment.
If a code has delegation prefix, SetCodeToEOA should be used for 7702.
There was a problem hiding this comment.
This can be done in test code. Or this is bug.
ulbqb
commented
Jan 22, 2025
| "github.com/kaiachain/kaia/blockchain/types" | ||
| "github.com/kaiachain/kaia/common" | ||
| "github.com/kaiachain/kaia/consensus/gxhash" | ||
| "github.com/stretchr/testify/suite" | ||
| ) | ||
|
|
||
| func TestBlockchain(t *testing.T) { |
There was a problem hiding this comment.
I think it's ok to remove this.
Mdaiki0730
reviewed
Jan 22, 2025
| @@ -223,7 +224,7 @@ func (tm *testMatcher) runTestFile(t *testing.T, path, name string, runTest inte | |||
| t.Skip("Skipped by whitelist") | |||
| } | |||
| } | |||
| t.Parallel() | |||
| // t.Parallel() | |||
There was a problem hiding this comment.
Nice to have: Enable this
| @@ -159,11 +199,13 @@ See https://github.com/ethereum/tests/wiki/Blockchain-Tests-II | |||
| expected we are expected to ignore it and continue processing and then validate the | |||
| post state. | |||
| */ | |||
| func (t *BlockTest) insertBlocks(blockchain *blockchain.BlockChain) ([]btBlock, error) { | |||
| func (t *BlockTest) insertBlocks(bc *blockchain.BlockChain, preBlock *types.Block) ([]btBlock, error) { | |||
There was a problem hiding this comment.
We may not need to change insertBlock since it is no longer used.
We can also consider deleting it.
|
|
||
| // Because it is a eth test, we don't have to think about fee payer | ||
| // Because the baseFee is set to 0, Kaia's gas fee may be 0 if the transaction has a dynamic fee. | ||
| senderMap[tx.ValidatedSender()] = new(big.Int).Sub( |
There was a problem hiding this comment.
If tx.ValidatedSender() is already present in the map, you must add it to it.
| } | ||
|
|
||
| // Modify the decode function | ||
| func (bb *btBlock) decode(latestParentHash common.Hash, latestRoot common.Hash) (*types.Block, error) { |
There was a problem hiding this comment.
It seems that decode is no longer necessary.
| } | ||
|
|
||
| // Decode header | ||
| var header TestHeader |
ulbqb
commented
Jan 23, 2025
Comment on lines
+2770
to
+2772
| if GasLimitInExecutionSpecTest != 0 { | ||
| blockContext.GasLimit = GasLimitInExecutionSpecTest | ||
| } |
There was a problem hiding this comment.
First, it's correct that Kaia's GasLimit is zero?
ulbqb
commented
Jan 23, 2025
Comment on lines
+2777
to
+2783
|
|
||
| // when execution spec test, we can insert test GasLimit to blockContext. | ||
| if UseKaiaCancunExtCodeHashFee && chainConfig.Rules(header.Number).IsCancun { | ||
| // EIP-1052 must be activated for backward compatibility on Kaia. But EIP-2929 is activated instead of it on Ethereum | ||
| vm.ChangeGasCostForTest(&vmenv.Config.JumpTable, vm.EXTCODEHASH, params.WarmStorageReadCostEIP2929) | ||
| } | ||
|
|
There was a problem hiding this comment.
This can be implemented in test code.
ulbqb
commented
Jan 23, 2025
| @@ -316,7 +317,7 @@ func (t *TxInternalDataFeeDelegatedValueTransfer) SenderTxHash() common.Hash { | |||
| } | |||
|
|
|||
| func (t *TxInternalDataFeeDelegatedValueTransfer) Validate(stateDB StateDB, currentBlockNumber uint64) error { | |||
| if common.IsPrecompiledContractAddress(t.Recipient) { | |||
| if common.IsPrecompiledContractAddress(t.Recipient, *fork.Rules(big.NewInt(int64(currentBlockNumber)))) { | |||
There was a problem hiding this comment.
This can be removed, executing test for each fork?
ulbqb
commented
Jan 23, 2025
Comment on lines
+72
to
+80
| func (suite *ExecutionSpecBlockTestSuite) TearDownSuite() { | ||
| // Reset global variables for test | ||
| common.IsPrecompiledContractAddress = suite.originalIsPrecompiledContractAddress | ||
| blockchain.UseKaiaCancunExtCodeHashFee = false | ||
| blockchain.GasLimitInExecutionSpecTest = 0 | ||
| blockchain.CreateContractWithCodeFormatInExecutionSpecTest = false | ||
| types.IsPragueInExecutionSpecTest = false | ||
| gxhash.CustomInitialize = nil | ||
| } |
There was a problem hiding this comment.
Using build tag, can these be switched?
ulbqb
commented
Jan 23, 2025
| if gblock.Hash() != t.json.Genesis.Hash { | ||
| return fmt.Errorf("genesis block hash doesn't match test: computed=%x, test=%x", gblock.Hash().Bytes()[:6], t.json.Genesis.Hash[:6]) | ||
|
|
||
| st := MakePreState(db, t.json.Pre, true, config.Rules(gblock.Number())) |
There was a problem hiding this comment.
After this, can we overwrite pre state?
ulbqb
commented
Jan 23, 2025
Comment on lines
+119
to
+128
| config.SetDefaults() | ||
| // Since we calculate the baseFee differently than eth, we will set it to 0 to turn off the gas fee. | ||
| config.Governance.KIP71 = ¶ms.KIP71Config{ | ||
| LowerBoundBaseFee: 0, | ||
| UpperBoundBaseFee: 0, | ||
| GasTarget: 0, | ||
| MaxBlockGasUsedForBaseFee: 0, | ||
| BaseFeeDenominator: 0, | ||
| } | ||
| blockchain.InitDeriveSha(config) |
There was a problem hiding this comment.
I want to know how config is. Description of blockchain config is needed.
ulbqb
commented
Jan 23, 2025
| ethReward: ethReward, | ||
| } | ||
|
|
||
| i, err := bc.InsertChain(blocks) |
There was a problem hiding this comment.
Is the same block executed twice?
ulbqb
commented
Jan 23, 2025
Comment on lines
+377
to
+381
| if rewardList, exist := rewardMap[addr]; exist { | ||
| // In the case of rewardBaseAddress, the Kaia reward will be deducted once. | ||
| statedb.SubBalance(addr, rewardList.kaiaReward) | ||
| statedb.AddBalance(addr, rewardList.ethReward) | ||
| } |
There was a problem hiding this comment.
This should be run before loop.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is under refactoring.
refactoring Tasks: