-
Notifications
You must be signed in to change notification settings - Fork 21.1k
core/types: add block-level access list structures with encoding/decoding #31948
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
Conversation
I'm going to add RLP encoding/decoding to this so that we can evaluate the BALs produced by RLP vs SSZ. |
core/types/bal.go
Outdated
// accountAccess contains post-block account state for mutations as well as | ||
// all storage keys that were read during execution. | ||
type accountAccess struct { | ||
StorageWrites storageWrites `json:"storageWrites,omitempty"` |
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.
The EIP calls them storage_changes
core/types/bal_encoding.go
Outdated
StorageReads [][32]byte `ssz-max:"300000"` | ||
BalanceChanges []encodingBalanceChange `ssz-max:"300000"` | ||
NonceChanges []encodingAccountNonce `ssz-max:"300000"` | ||
Code []encodingCodeChange `ssz-max:"1"` |
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.
Also needs to have a max of 30_000 according to spec (maybe the spec should be updated 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.
I think this is probably a typo in the spec (I will PR a change to the EIP). The max should be 1: after selfdestruct removal, the code of an account can only change once (we don't include delegations in the BAL because they can be determined statically by looking at the block txs).
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 it? I thought with the way selfdestruct is specced now, you can update the contract multiple times in initcode
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.
I guess you would only store the code after the tx was executed, so yeah you're right
core/types/bal_test.go
Outdated
if err := dec.DecodeRLP(rlp.NewStream(bytes.NewReader(buf.Bytes()), 10000000)); err != nil { | ||
t.Fatalf("decoding failed: %v\n", err) | ||
} | ||
if dec.Hash() != b.Hash() { |
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 only checks the rlp encoding/decoding, we should also test the ssz
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.
I think we should add a fuzzer, especially for the conversion from BAL to BAL_Encoding and back
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.
The RLP format of the access list is the SSZ-encoded access list wrapped into RLP bytes (that's how it would be encoded into the block body). So encoding/decoding the RLP performs SSZ encoding/decoding under the hood.
core/types/bal.go
Outdated
return res | ||
} | ||
|
||
func (e *encodingBlockAccessList) prettyPrint() string { |
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 should probably be moved? or not be the encodingBlockAccessList
type
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.
It's called by BlockAccess.PrettyPrint
: I convert to the encoding format before printing so that the output of PrettyPrint
is consistent, instead of random iteration order over maps in the BlockAccessList
.
core/types/bal.go
Outdated
@@ -0,0 +1,227 @@ | |||
package types |
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.
header
core/types/bal_encoding.go
Outdated
@@ -0,0 +1,361 @@ | |||
package types |
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.
header
core/types/bal_test.go
Outdated
@@ -0,0 +1,92 @@ | |||
package types |
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.
header
I added generated RLP encoders/decoders. I'm thinking that it doesn't make sense to merge this PR until we determine which serialization format to move forward with. I will do some analysis, generating BALs from a large historical block range with both RLP and SSZ, and we can compare the results. |
Started prototyping BAL execution. I think that there's going to be quite a few changes to this PR as a result. |
core/types/bal: polish
… to BlockAccessList
…ode the BALs directly)
This adds the SSZ types from the EIP-7928 and also adds encoder/decoder generation using https://github.com/ferranbt/fastssz.
The fastssz dependency is updated because the generation will not work properly with the master branch version due to a bug in fastssz.