-
Notifications
You must be signed in to change notification settings - Fork 419
feat(tests): adds EIP-7981 test and required framework changes #2144
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
base: eips/amsterdam/eip-7981
Are you sure you want to change the base?
Changes from 1 commit
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 | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -569,6 +569,18 @@ def validate_transaction(tx: Transaction) -> Tuple[Uint, Uint]: | |||||||||||||||||||||||||||||||||
| return intrinsic_gas, calldata_floor_gas_cost | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| def count_tokens_in_data(data: bytes) -> Uint: | ||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||
| Count the tokens in an arbitrary input data. | ||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||
| zero_bytes = 0 | ||||||||||||||||||||||||||||||||||
| for byte in data: | ||||||||||||||||||||||||||||||||||
| if byte == 0: | ||||||||||||||||||||||||||||||||||
| zero_bytes += 1 | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| return Uint(zero_bytes + (len(data) - zero_bytes) * 4) | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
| return Uint(zero_bytes + (len(data) - zero_bytes) * 4) | |
| return Uint(zero_bytes + (len(data) - zero_bytes) * CALLDATA_TOKENS_PER_NONZERO_BYTE) |
?
Outdated
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.
If we're iterating through all of data anyway, how about:
| zero_bytes = 0 | |
| for byte in data: | |
| if byte == 0: | |
| zero_bytes += 1 | |
| return Uint(zero_bytes + (len(data) - zero_bytes) * 4) | |
| return sum(Uint(4) if byte else Uint(1) for byte in data) |
Or if that's too Pythonic:
| zero_bytes = 0 | |
| for byte in data: | |
| if byte == 0: | |
| zero_bytes += 1 | |
| return Uint(zero_bytes + (len(data) - zero_bytes) * 4) | |
| return sum(Uint(1) if byte == 0 else Uint(4) for byte in data) |
Outdated
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 be backported to previous forks.
Outdated
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.
We should avoid putting constant values in docstrings. It's too easy to update the code and forget to update the docs. Instead, link to the relevant constant.
Outdated
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'm not sure we should establish the pattern of commenting with an EIP number. If we take this approach, we'll end up with piles of comments where a block of code is introduced under EIP-X, then in a later fork EIP-Y will modify that block, making it misleading which EIP a sequence of lines belongs to. For example:
| Fork T | Fork T+1 |
|---|---|
# EIP-1234: The dingus is the rate of fleep
dingus = a + b
dingus += c ^ d
dingus /= fleep(e) |
# EIP-1234: The dingus is the rate of fleep
dingus = a + b
# EIP-4567: Frobulate the dingus
dingus = frobulate(dingus)
dingus += c ^ d # <-
dingus /= fleep(e) # <- |
The marked lines (<-) are now incorrectly attributed to EIP-4567 in Fork+1. Instead, I'd recommend omitting the EIP identifier in the comments, and instead describe the changes introduced by the EIP in the function's docstrings. The rendered diffs will make it pretty obvious what's changed.
Perhaps I'm being overly cautious, however.
Outdated
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.
You'll need to backport the comment as well.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| """Tests for EIP-7981: Increase Access List Cost.""" |
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.
Should the
1be in a named constant? Something likeCALLDATA_TOKENS_PER_ZERO_BYTE?