feat: support create_access_list #768
Conversation
thomas-nguy
requested review from
XinyuCRO and
randy-cro
and removed request for
a team
This comment has been minimized.
This comment has been minimized.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #768 +/- ##
===========================================
+ Coverage 46.85% 46.86% +0.01%
===========================================
Files 189 189
Lines 15351 15477 +126
===========================================
+ Hits 7193 7254 +61
- Misses 7360 7417 +57
- Partials 798 806 +8
🚀 New features to boost your workflow:
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
thomas-nguy
force-pushed
the
thomas/access-list-support
branch
from
9eae520 to
f4606c5
Compare
| args.Value = new(hexutil.Big) | ||
| } | ||
| if args.Nonce == nil { | ||
| if args.From == nil { |
There was a problem hiding this comment.
why is this check here within the nonce check?
There was a problem hiding this comment.
line below requires non null
nonce, _ := b.getAccountNonce(*args.From, true, 0, b.logger)
| client.On("ABCIQueryWithOptions", context.Background(), mock.Anything, mock.Anything, mock.Anything). | ||
| Return(&tmrpctypes.ResultABCIQuery{ | ||
| Response: abci.ResponseQuery{ | ||
| Value: []byte{2}, // TODO replace with data.Bytes(), |
There was a problem hiding this comment.
yeah not sure, just copying from above
| queryClient.On("Params", rpc.ContextWithHeight(height), &evmtypes.QueryParamsRequest{}). | ||
| Return(nil, errortypes.ErrInvalidRequest) | ||
| Return(nil, errortypes.ErrInvalidRequest). | ||
| Maybe() |
There was a problem hiding this comment.
why is Maybe() added for all these test? will it make the test cases less robust?
There was a problem hiding this comment.
it just avoid throwing error when those mocks are not called. It add more flexibility.
I am adding them to that we can create a common method to mock the whole backend and tht can be reused across tests.
It may make the existing tests less robust but I think we should always verify the end results anyways, and not just relying on the mock being called
| } | ||
|
|
||
| isMerge := b.ChainConfig().MergeNetsplitBlock != nil | ||
| precompiles := vm.ActivePrecompiles(b.ChainConfig().Rules(header.Number, isMerge, header.Time)) |
There was a problem hiding this comment.
do we also need to excludepreInstalls?
There was a problem hiding this comment.
following go-ethereum
https://github.com/ethereum/go-ethereum/blob/master/internal/ethapi/api.go#L1278
tldr precompiles are already warmed, so no need to add them. Preinstall are just normal contracts and should not be exluded
Signed-off-by: Thomas <[email protected]>
| return nil, 0, nil, err | ||
| } | ||
|
|
||
| prevTracer, traceArgs, err := b.initAccessListTracer(args, blockNum, addressesToExclude) |
There was a problem hiding this comment.
I don't find the location that prevTracer wired into the EVM execution.
3 participants
Closes: #XXX
Description
For contributor use:
docs/) or specification (x/<module>/spec/)godoccomments.Unreleasedsection inCHANGELOG.mdFiles changedin the Github PR explorerFor admin use:
WIP,R4R,docs, etc)