-
Notifications
You must be signed in to change notification settings - Fork 3
feat(core/types): Block
RLP overriding
#133
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
Changes from all commits
b6241e3
756068e
843bf2f
5103121
dc35c47
9e3a464
7accc33
7948b43
04ead3c
1e49ad6
e5b3c5d
29901b5
927ac8b
59d6682
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -211,6 +211,8 @@ type Block struct { | |
// inter-peer block relay. | ||
ReceivedAt time.Time | ||
ReceivedFrom interface{} | ||
|
||
extra *pseudo.Type // See [RegisterExtras] | ||
} | ||
|
||
// "external" block encoding. used for eth protocol, etc. | ||
|
@@ -219,6 +221,8 @@ type extblock struct { | |
Txs []*Transaction | ||
Uncles []*Header | ||
Withdrawals []*Withdrawal `rlp:"optional"` | ||
|
||
hooks BlockBodyHooks // libevm: MUST be unexported + populated from [Block.hooks] | ||
} | ||
|
||
// NewBlock creates a new block. The input data is copied, changes to header and to the | ||
|
@@ -318,6 +322,7 @@ func CopyHeader(h *Header) *Header { | |
// DecodeRLP decodes a block from RLP. | ||
func (b *Block) DecodeRLP(s *rlp.Stream) error { | ||
var eb extblock | ||
eb.hooks = b.hooks() | ||
_, size, _ := s.Kind() | ||
if err := s.Decode(&eb); err != nil { | ||
return err | ||
|
@@ -334,13 +339,14 @@ func (b *Block) EncodeRLP(w io.Writer) error { | |
Txs: b.transactions, | ||
Uncles: b.uncles, | ||
Withdrawals: b.withdrawals, | ||
hooks: b.hooks(), | ||
}) | ||
} | ||
|
||
// Body returns the non-header content of the block. | ||
// Note the returned data is not an independent copy. | ||
func (b *Block) Body() *Body { | ||
return &Body{b.transactions, b.uncles, b.withdrawals, nil /* unexported extras field */} | ||
return &Body{b.transactions, b.uncles, b.withdrawals, b.cloneExtra()} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do we want to clone? Wondering given we don't really deep copy any other field There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here's my thinking: I've been bitten really hard in the past by bugs caused by a lack of it (queue 📣 🦀) so by default I tend to give my API users at least a choice or signal. Forcing them to have a pointer payload is only a subtle hint while the use of Not including it feels like setting a trap for a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Deep copy is great, especially by default. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This doesn't force a deep copy on the user as they're free to return the original. Any performance issues here, measured per block, are almost certainly dwarfed by those measured per storage R/W operation. As an example, I benchmarked the impact of a change from the I think we're far more likely to shoot ourselves in the foot due to a missed copy than we are to take a meaningful performance hit, and we can always revisit performance while we aren't guaranteed the opportunity to revisit a catastrophic bug. |
||
} | ||
|
||
// Accessors for body data. These do not return a copy because the content | ||
|
@@ -458,6 +464,7 @@ func (b *Block) WithSeal(header *Header) *Block { | |
transactions: b.transactions, | ||
uncles: b.uncles, | ||
withdrawals: b.withdrawals, | ||
extra: b.cloneExtra(), | ||
ARR4N marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
} | ||
|
||
|
@@ -468,6 +475,7 @@ func (b *Block) WithBody(body Body) *Block { | |
transactions: make([]*Transaction, len(body.Transactions)), | ||
uncles: make([]*Header, len(body.Uncles)), | ||
withdrawals: b.withdrawals, | ||
extra: body.cloneExtra(), | ||
} | ||
copy(block.transactions, body.Transactions) | ||
for i := range body.Uncles { | ||
|
@@ -482,6 +490,7 @@ func (b *Block) WithWithdrawals(withdrawals []*Withdrawal) *Block { | |
header: b.header, | ||
transactions: b.transactions, | ||
uncles: b.uncles, | ||
extra: b.cloneExtra(), | ||
ARR4N marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
if withdrawals != nil { | ||
block.withdrawals = make([]*Withdrawal, len(withdrawals)) | ||
|
Uh oh!
There was an error while loading. Please reload this page.