fix modexp bug: return 0 if base is zero #6424
Conversation
|
It looks like @Hawstein signed our Contributor License Agreement. 👍 Many thanks, Parity Technologies CLA Bot |
|
indeed |
ethcore/src/builtin.rs
Outdated
| @@ -309,7 +309,7 @@ impl Impl for ModexpImpl { | |||
| base = base % &modulus; | |||
|
|
|||
| // fast path for base divisible by modulus. | |||
| if base.is_zero() { return result } | |||
| if base.is_zero() { return BigUint::zero() } | |||
There was a problem hiding this comment.
There was a problem hiding this comment.
No. the place you mentioned is the original base. But here the base is calculated via base % &modulus. See:
https://github.com/paritytech/parity/pull/6424/files#diff-f01cea79b5ea6391c260249a4cf801a5R309
There was a problem hiding this comment.
Ah, right. A test would be good
There was a problem hiding this comment.
But I think we can just move base = base % &modulus; up to the first line in this function. and remove the test here. All the condition will be tested at:
https://github.com/paritytech/parity/pull/6424/files#diff-f01cea79b5ea6391c260249a4cf801a5R303
There was a problem hiding this comment.
Then it would panic if the modulo was zero, which is protected at the given point.
There was a problem hiding this comment.
I am struggling for the test. There are more stuffs besides the modexp function. I can easily test the modexp function, however since it is just a inner function, I have to test the ModexpImpl::execute. I will be really appreciated if there are some instructions.
There was a problem hiding this comment.
oh just move the function outside to the mod level and add a test, there are plenty of support functions there already
|
waiting for a test. @rphmeier note the end of the conversation:
|
gavofyork
added
A5-grumble 🔥
Hawstein
force-pushed
the
fix-modexp-bug
branch
from
0f02218
to
4794124
Compare
|
@gavofyork @NikVolf The PR is updated according to your suggestion. |
NikVolf
added
A8-looksgood 🦄
It currently returns
resultwhich isBigUint::one().