-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Implement EIP-2565 (option B) #11728
base: master
Are you sure you want to change the base?
Conversation
|
By "Option B", do you mean option 2? |
Yes |
48caef7 to
7cdba58
Compare
7b4b04e to
d2123e3
Compare
d2123e3 to
1573eb7
Compare
ineffectualproperty
left a comment
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.
Provided some suggested changes given the modifications to EIP-2565
| "info": "EIP 2565 transition at block X", | ||
| "price": { | ||
| "modexp2": { | ||
| "divisor": 20 |
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.
| "divisor": 20 | |
| "divisor": 3 |
| let (gas, overflow) = (complexity_formula)(m).overflowing_mul(max(adjusted_exp_len, 1)); | ||
| if overflow { | ||
| return U256::max_value(); | ||
| } |
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.
(gas / self.divisor as u64).into() --> min((gas / self.divisor as u64).into(),100)
This sets a minimum gas price of 100
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.
FYI this is a couple rows down but it wouldn't let me comment on it for some reason
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.
EIP has been updated to set the minimum gas price at 200
| Pricing::Modexp(ModexpPricer { | ||
| divisor: if exp.divisor == 0 { | ||
| warn!(target: "builtin", "Zero modexp divisor specified. Falling back to default: 10."); | ||
| 10 |
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.
| 10 | |
| 3 |
|
EIP has been updated in this PR: ethereum/EIPs#2892 |
| } | ||
|
|
||
| fn mult_complexity_new(x: u64) -> u64 { | ||
| ((x / 64) + if x % 64 == 0 { 0 } else { 1 }) ^ 2 |
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.
Replace 64 with 8 to represent limbs in bytes rather than bits per most recent EIP update
This PR implements EIP-2565: Repricing of the EIP-198 ModExp precompile, more specifically Option 2. It introduces a new computational complexity formula which should better align with the real computational costs for OpenEthereum and Geth.
No test vectors yet, so still a draft PR.