-
Notifications
You must be signed in to change notification settings - Fork 5.7k
Update EIP-6110: Spec parse_deposit_data
#9460
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 6 commits
232872a
8e67988
9b687a3
4f912ac
3c96c46
9bef0b9
eb150ed
01e54fc
194f66e
43cf243
b1e69fd
31641e8
6b0eb53
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 |
---|---|---|
|
@@ -76,11 +76,35 @@ Beginning with the `FORK_BLOCK`, each deposit accumulated in the block **MUST** | |
in the order they appear in the logs. To illustrate: | ||
|
||
```python | ||
def parse_deposit_data(deposit_event_data) -> bytes[]: | ||
""" | ||
Parses deposit data from DepositContract.DepositEvent data | ||
""" | ||
pass | ||
def parse_deposit_data(deposit_event_data): | ||
""" | ||
Parses raw emitted EVM log data to decode the request data | ||
""" | ||
|
||
|
||
# These offsets points are data pointers which first encode the | ||
# size of the data (which is always 32 bytes) | ||
# The data itself (of this size) is then appended after the size 32 bytes | ||
PUBKEY_OFFSET = deposit_event_data[0:32] | ||
WITHDRAWAL_CREDENTIALS_OFFSET = deposit_event_data[32:64] | ||
AMOUNT_OFFSET = deposit_event_data[64:96] | ||
SIGNATURE_OFFSET = deposit_event_data[96:128] | ||
INDEX_OFFSET = deposit_event_data[128:160] | ||
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. What if any of these offsets or derived values is outside of the actual data length? 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. Yes, good point, I will add in that if this is the case (the LOG is thus invalid) then it is thus also not a valid request and should be skipped 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 does have the complication that it's "system tx" level so will fail the block (as per Sepolia) Which flows into the other discussion about what happens when a system tx fails 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. In this case it meant the contract produced an invalid log data thus invalidating a block would be reasonable. |
||
|
||
# These sizes are the sizes of the relevant data | ||
PUBKEY_SIZE = deposit_event_data[PUBKEY_OFFSET:PUBKEY_OFFSET+32] | ||
WITHDRAWAL_CREDENTIALS_SIZE = deposit_event_data[WITHDRAWAL_CREDENTIALS_OFFSET:WITHDRAWAL_CREDENTIALS_OFFSET+32] | ||
AMOUNT_SIZE = deposit_event_data[AMOUNT_OFFSET:AMOUNT_OFFSET+32] | ||
SIGNATURE_SIZE = deposit_event_data[SIGNATURE_OFFSET:SIGNATURE_OFFSET+32] | ||
INDEX_SIZE = deposit_event_data[INDEX_OFFSET:INDEX_OFFSET+32] | ||
|
||
pubkey = deposit_event_data[PUBKEY_OFFSET + 32:PUBKEY_OFFSET + 32 + PUBKEY_SIZE] | ||
withdrawal_credentials = deposit_event_data[WITHDRAWAL_CREDENTIALS_OFFSET + 32:WITHDRAWAL_CREDENTIALS_OFFSET + 32 + WITHDRAWAL_CREDENTIALS_SIZE] | ||
amount = deposit_event_data[AMOUNT_OFFSET + 32:AMOUNT_OFFSET + 32 + AMOUNT_SIZE] | ||
signature = deposit_event_data[SIGNATURE_OFFSET + 32:SIGNATURE_OFFSET + 32 + SIGNATURE_SIZE] | ||
index = deposit_event_data[INDEX_OFFSET + 32:INDEX_OFFSET + 32 + INDEX_SIZE] | ||
|
||
return [pubkey, withdrawal_credentials, amount, signature, index] | ||
|
||
def event_data_to_deposit_request(deposit_event_data) -> bytes: | ||
deposit_data = parse_deposit_data(deposit_event_data) | ||
|
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.
Since this is not constant anymore, it shouldn’t be capital case. Plus, i think we should parse an offset as
int
(same for len)