Create access list #22550
Merged
Create access list #22550
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
MariusVanDerWijden
requested review from
holiman,
karalabe and
rjl493456442
as code owners
holiman
reviewed
Mar 22, 2021
holiman
reviewed
Mar 22, 2021
4 tasks
So, in the test, this is an invalid opcode. But it could have been a failure caused by |
|
So you would prefer something like this? The problem is that the sender will not know whether the createAccessList succeeded, or if they just created an access list for an always failing transaction |
|
I think I'd prefer if the result told me both the accumulated accesslist and whether the tx succeeded. |
|
It should also tell me what the gas usage was on successfull txs. So I can spot the difference if I send it in explicitlly or of it becomes auto-populated (1900 vs 2100 per account, IIRC) |
|
Wdym with successful transactions? If we found a tx in the loop that was successful, even with an incomplete access list? |
I just meant that |
|
|
I pushed some fixes, to handle the panic and also make the output gasUsed same format as the input Why doesn't it tell me that |
|
Hm, you do set it, but it later becomes forgotten: |
|
Yeah, so the tracer doesn't know about the existing one. You'll have to merge the output from the tracer and the input accesslist in every iteration. Or init the tracer with the input data. |
MariusVanDerWijden
force-pushed
the
create_access_list
branch
from
acb266b
to
dc9a7b1
Compare
|
|
There's quite a lot of switching back and forth between whether it's a pointer or not. Seems like maybe the code could be simpler by not having it be a pointer: diff --git a/core/vm/access_list_tracer.go b/core/vm/access_list_tracer.go
index b7ed80ef5e..49b7622bce 100644
--- a/core/vm/access_list_tracer.go
+++ b/core/vm/access_list_tracer.go
@@ -85,7 +85,7 @@ func (a *accessList) Copy() *accessList {
return cp
}
-func (a *accessList) ToAccessList() *types.AccessList {
+func (a *accessList) ToAccessList() types.AccessList {
acl := make([]types.AccessTuple, 0, len(a.addresses))
for addr, idx := range a.addresses {
var tuple types.AccessTuple
@@ -101,21 +101,19 @@ func (a *accessList) ToAccessList() *types.AccessList {
acl = append(acl, tuple)
}
cast := types.AccessList(acl)
- return &cast
+ return cast
}
type AccessListTracer struct {
list *accessList
}
-func NewAccessListTracer(acl *types.AccessList) *AccessListTracer {
+func NewAccessListTracer(acl types.AccessList) *AccessListTracer {
list := newAccessList()
- if acl != nil {
- for _, al := range *acl {
- list.AddAddress(al.Address)
- for _, slot := range al.StorageKeys {
- list.AddSlot(al.Address, slot)
- }
+ for _, al := range acl {
+ list.AddAddress(al.Address)
+ for _, slot := range al.StorageKeys {
+ list.AddSlot(al.Address, slot)
}
}
return &AccessListTracer{
@@ -150,11 +148,11 @@ func (*AccessListTracer) CaptureFault(env *EVM, pc uint64, op OpCode, gas, cost
func (*AccessListTracer) CaptureEnd(output []byte, gasUsed uint64, t time.Duration, err error) {}
-func (a *AccessListTracer) GetAccessList() *types.AccessList {
+func (a *AccessListTracer) GetAccessList() types.AccessList {
return a.list.ToAccessList()
}
-func (a *AccessListTracer) GetUnpreparedAccessList(sender common.Address, dst *common.Address, precompiles []common.Address) *types.AccessList {
+func (a *AccessListTracer) GetUnpreparedAccessList(sender common.Address, dst *common.Address, precompiles []common.Address) types.AccessList {
copy := a.list.Copy()
copy.DeleteAddressIfNoSlotSet(sender)
if dst != nil {
diff --git a/eth/api.go b/eth/api.go
index 6b90d8eb03..8d332ea885 100644
--- a/eth/api.go
+++ b/eth/api.go
@@ -545,7 +545,7 @@ func (api *PrivateDebugAPI) getModifiedAccounts(startBlock, endBlock *types.Bloc
}
type AccessListResult struct {
- Accesslist *types.AccessList `json:"accessList"`
+ Accesslist types.AccessList `json:"accessList"`
Error string `json:"error,omitempty"`
GasUsed hexutil.Uint64 `json:"gasUsed"`
}
diff --git a/eth/api_backend.go b/eth/api_backend.go
index a86d7266d1..859a381a98 100644
--- a/eth/api_backend.go
+++ b/eth/api_backend.go
@@ -351,7 +351,7 @@ func (b *EthAPIBackend) StateAtTransaction(ctx context.Context, block *types.Blo
// AccessList creates an access list for the given transaction.
// If the accesslist creation fails an error is returned.
// If the transaction itself fails, an vmErr is returned.
-func (b *EthAPIBackend) AccessList(ctx context.Context, block *types.Block, reexec uint64, args *ethapi.SendTxArgs) (acl *types.AccessList, gasUsed uint64, vmErr error, err error) {
+func (b *EthAPIBackend) AccessList(ctx context.Context, block *types.Block, reexec uint64, args *ethapi.SendTxArgs) (acl types.AccessList, gasUsed uint64, vmErr error, err error) {
if block == nil {
block = b.CurrentBlock()
}
@@ -368,19 +368,18 @@ func (b *EthAPIBackend) AccessList(ctx context.Context, block *types.Block, reex
}
var (
gas uint64
- accessList = args.AccessList
+ accessList types.AccessList
msg types.Message
)
+ if args.AccessList != nil{
+ accessList = *args.AccessList
+ }
for i := 0; i < 10; i++ {
log.Trace("Creating Access list", "accesslist", accessList)
// Copy the original db so we don't modify it
statedb := db.Copy()
// If we have an accesslist, use it
- if accessList != nil {
- msg = types.NewMessage(args.From, args.To, uint64(*args.Nonce), args.Value.ToInt(), uint64(*args.Gas), args.GasPrice.ToInt(), *args.Data, *accessList, false)
- } else {
- msg = types.NewMessage(args.From, args.To, uint64(*args.Nonce), args.Value.ToInt(), uint64(*args.Gas), args.GasPrice.ToInt(), *args.Data, nil, false)
- }
+ msg = types.NewMessage(args.From, args.To, uint64(*args.Nonce), args.Value.ToInt(), uint64(*args.Gas), args.GasPrice.ToInt(), *args.Data, accessList, false)
// Apply the transaction
context := core.NewEVMBlockContext(block.Header(), b.eth.blockchain, nil)
tracer := vm.NewAccessListTracer(accessList)
|
holiman
reviewed
Mar 25, 2021
core/vm/access_list_tracer.go
Outdated
| } | ||
|
|
||
| func (a *accessList) ToAccessList() *types.AccessList { | ||
| acl := make([]types.AccessTuple, 0, len(a.addresses)) |
There was a problem hiding this comment.
Suggested change
| acl := make([]types.AccessTuple, 0, len(a.addresses)) | |
| acl := make(types.AccessList, 0, len(a.addresses)) |
And then no cast is needed, just return it
There was a problem hiding this comment.
Another idea would be to make state.accessList public. Although I do think it's kind of nice to have the tracer be standalone, even if it does introduce this copy-paste struct. @fjl what do you think?
There was a problem hiding this comment.
Yeah I thought about that too, but I think having two public structs called AccessList would probably confuse a lot of people, especially since it is only used as an internal datastructure
|
Oh, one more thing: If the |
karalabe
force-pushed
the
create_access_list
branch
from
b4753ef
to
ce89aaf
Compare
holiman
reviewed
Apr 7, 2021
core/vm/access_list_tracer.go
Outdated
| idx, addrPresent := al.addresses[address] | ||
| if !addrPresent || idx == -1 { | ||
| delete(al.addresses, address) | ||
| func (al accessList) deleteAddressIfNoSlotSet(address common.Address) { |
There was a problem hiding this comment.
Do you still need this method?
karalabe
force-pushed
the
create_access_list
branch
from
ce89aaf
to
05f6399
Compare
Would be neat if storagekeys defaults to empty list if not specified... |
atif-konasl
pushed a commit
to frozeman/pandora-execution-engine
that referenced
this pull request
Oct 15, 2021
* core/vm: implement AccessListTracer * eth: implement debug.createAccessList * core/vm: fixed nil panics in accessListTracer * eth: better error messages for createAccessList * eth: some fixes on CreateAccessList * eth: allow for provided accesslists * eth: pass accesslist by value * eth: remove created acocunt from accesslist * core/vm: simplify access list tracer * core/vm: unexport accessListTracer * eth: return best guess if al iteration times out * eth: return best guess if al iteration times out * core: docstring, unexport methods * eth: typo * internal/ethapi: move createAccessList to eth package * internal/ethapi: remove reexec from createAccessList * internal/ethapi: break if al is equal to last run, not if gas is equal * internal/web3ext: fixed arguments * core/types: fixed equality check for accesslist * core/types: no hardcoded vals * core, internal: simplify access list generation, make it precise * core/vm: fix typo Co-authored-by: Martin Holst Swende <martin@swende.se> Co-authored-by: Péter Szilágyi <peterke@gmail.com>
9 tasks
This was referenced Sep 23, 2022
9 tasks
18 tasks
gzliudan
added a commit
to gzliudan/XDPoSChain
that referenced
this pull request
May 14, 2024
19 tasks
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.
replaces #22289
depends on #22333
TODO: