Add Math.modExp and a Panic library

[mw2000]

Fixes #1985
Introduces the wrapper library for EIP 198 that @frangio mentioned in issue #1985. Code was taken from https://ethereum.stackexchange.com/questions/71565/verifying-modular-exponentiation-operation-in-etherum

Looking forward to code review and comments to add docs and Changelog entry.

PR Checklist

• Tests
• Documentation
• Changelog entry

[changeset-bot]

🦋 Changeset detected

Latest commit: f352681

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[mw2000]

⚠️ No Changeset found

Latest commit: 0d78d29

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Added the changeset in next commit

[mw2000]

@frangio had been a while since I worked on this repo, but here's some updated code if it's worth merging in the next update

[ernestognw]

For what it's worth, EVM Codes says the same thing about the DIV and MOD opcodes, but the high level operators in Solidity do panic.

Yeah, that's right! I'm fine defaulting to Solidity's behavior if it's a "generic" error. Note that we agreed to add a Panic library, although this still needs some work.

My 2 cents are that we should revert by default, following the footsteps of % in SafeMath/Solidity. But I'll also note that we have the try* pattern that we could reuse here, i.e. offer tryModExp that returns false without reverting, and modExp that does revert on error

Right. I'll try to implement it in that way

[ernestognw]

A potential threat here is that in chains where this precompile is not available, then the result length will be 0.

I guess that's the most reliable way to protect against but I don't think it's worth the check.

I added a not specifying that developers need to make sure if the precompile is available.

[ernestognw]

I couldn't find a way in which calling the precompile returns false. I wonder if that's even possible. In such case the success flag is not even needed.

[Amxx]

In such case the success flag is not even needed.

Success MUST return false if the modulo is 0. Just like tryDiv returns false if the denominator is 0. There shouldn't even be a discussion about that. Dividing by 0 is an error, and taking the remainder of this division is also an error.

[Amxx]

I think this is good.

@ernestognw wdyt ?

Note that the coverage issue comes from the

} else {
    revert Address.FailedInnerCall();
}

which we don't know how to trigger.

I think its ok for use admin override for that.

[ernestognw]

I think this is good.

@ernestognw wdyt ?

Note that the coverage issue comes from the

} else {
    revert Address.FailedInnerCall();
}

which we don't know how to trigger.

I think its ok for use admin override for that.

Yeah it's good, just added an usage example and natspec to the panic function.

Also agree with the coverage drop for this, but I wouldn't remove it in case a weird chain has a contract in that address that does revert.

[ernestognw]

LGTM