-
Notifications
You must be signed in to change notification settings - Fork 12.1k
Add Math.modExp
and a Panic
library
#3298
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 59 commits
91e39eb
712a0f3
77f33ea
4683f26
7489ce4
d576641
0d78d29
5d99ed8
acb16f2
d7e81cf
85187dd
d458765
01badeb
a1c1439
d81d69d
6e8cced
fd7b8de
26a036e
4ba0a29
988c950
c08ac50
98f7994
f634aab
66a0c1f
eecd818
1583160
19ead8e
8cf355f
113e85e
4e1cf0d
6d7c154
84b285d
9e73f46
76c9afa
1ff0776
f84b1b6
fe32a38
4accc2e
cfd80e9
526d6b9
3718090
cd2f2e9
104002e
d149ea6
05aa60e
f352681
32ea4bb
2e962c8
275c959
24cd52a
50374a1
ee91836
d13e52d
969e259
e64c3f9
06220be
9137bae
81e8ba0
2b72050
2fc20d4
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 |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
'openzeppelin-solidity': minor | ||
--- | ||
|
||
`Math`: Add `modExp` function that exposes the `EIP-198` precompile. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
'openzeppelin-solidity': minor | ||
--- | ||
|
||
`Panic`: Add a library for reverting with panic codes. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
'openzeppelin-solidity': minor | ||
--- | ||
|
||
`Math`: MathOverflowedMulDiv error was replaced with native panic codes. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
// SPDX-License-Identifier: MIT | ||
|
||
pragma solidity ^0.8.20; | ||
|
||
import {Address} from "../utils/Address.sol"; | ||
import {Arrays} from "../utils/Arrays.sol"; | ||
import {Base64} from "../utils/Base64.sol"; | ||
import {BitMaps} from "../utils/structs/BitMaps.sol"; | ||
import {Checkpoints} from "../utils/structs/Checkpoints.sol"; | ||
import {Context} from "../utils/Context.sol"; | ||
import {Create2} from "../utils/Create2.sol"; | ||
import {DoubleEndedQueue} from "../utils/structs/DoubleEndedQueue.sol"; | ||
import {ECDSA} from "../utils/cryptography/ECDSA.sol"; | ||
import {EIP712} from "../utils/cryptography/EIP712.sol"; | ||
import {EnumerableMap} from "../utils/structs/EnumerableMap.sol"; | ||
import {EnumerableSet} from "../utils/structs/EnumerableSet.sol"; | ||
import {ERC165} from "../utils/introspection/ERC165.sol"; | ||
import {ERC165Checker} from "../utils/introspection/ERC165Checker.sol"; | ||
import {IERC165} from "../utils/introspection/IERC165.sol"; | ||
import {Math} from "../utils/math/Math.sol"; | ||
import {MerkleProof} from "../utils/cryptography/MerkleProof.sol"; | ||
import {MessageHashUtils} from "../utils/cryptography/MessageHashUtils.sol"; | ||
import {Multicall} from "../utils/Multicall.sol"; | ||
import {Nonces} from "../utils/Nonces.sol"; | ||
import {Panic} from "../utils/Panic.sol"; | ||
import {Pausable} from "../utils/Pausable.sol"; | ||
import {ReentrancyGuard} from "../utils/ReentrancyGuard.sol"; | ||
import {SafeCast} from "../utils/math/SafeCast.sol"; | ||
import {ShortStrings} from "../utils/ShortStrings.sol"; | ||
import {SignatureChecker} from "../utils/cryptography/SignatureChecker.sol"; | ||
import {SignedMath} from "../utils/math/SignedMath.sol"; | ||
import {StorageSlot} from "../utils/StorageSlot.sol"; | ||
import {Strings} from "../utils/Strings.sol"; | ||
import {Time} from "../utils/types/Time.sol"; | ||
|
||
abstract contract ExposeImports { | ||
// This will be transpiled, causing all the imports above to be transpiled when running the upgradeable tests. | ||
// This trick is necessary for testing libraries such as Panic.sol (which are not imported by any other transpiled | ||
// contracts and would otherwise not be exposed). | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,41 @@ | ||
// SPDX-License-Identifier: MIT | ||
|
||
pragma solidity ^0.8.20; | ||
|
||
/** | ||
* @dev Helper library for emitting standardized panic codes. | ||
* | ||
* Follows the list from libsolutil: https://github.com/ethereum/solidity/blob/v0.8.24/libsolutil/ErrorCodes.h | ||
*/ | ||
// slither-disable-next-line unused-state | ||
library Panic { | ||
/// @dev generic / unspecified error | ||
uint256 internal constant GENERIC = 0x00; | ||
/// @dev used by the assert() builtin | ||
uint256 internal constant ASSERT = 0x01; | ||
/// @dev arithmetic underflow or overflow | ||
uint256 internal constant UNDER_OVERFLOW = 0x11; | ||
/// @dev division or modulo by zero | ||
uint256 internal constant DIVISION_BY_ZERO = 0x12; | ||
/// @dev enum conversion error | ||
uint256 internal constant ENUM_CONVERSION_ERROR = 0x21; | ||
/// @dev invalid encoding in storage | ||
uint256 internal constant STORAGE_ENCODING_ERROR = 0x22; | ||
/// @dev empty array pop | ||
uint256 internal constant EMPTY_ARRAY_POP = 0x31; | ||
/// @dev array out of bounds access | ||
uint256 internal constant ARRAY_OUT_OF_BOUNDS = 0x32; | ||
/// @dev resource error (too large allocation or too large array) | ||
uint256 internal constant RESOURCE_ERROR = 0x41; | ||
/// @dev calling invalid internal function | ||
uint256 internal constant INVALID_INTERNAL_FUNCTION = 0x51; | ||
|
||
function panic(uint256 code) internal pure { | ||
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. Right now anyone can call I'm not sure how frequently Solidity adds (or changes) the panic codes, but imagine the following situation:
Wouldn't it be bad that the same revert code has different meanings? 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. Alternatively we can generate this library procedurally wth the codes, such that users can do:
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. I'm not too worried about users emitting random panic code ... but I'm not fundamentally opposed to making
We would have to make them CamelCase though. 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. The thing, if new panic code are ever added, the current version would allow users to use them without us having to do a release. 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.
Agree with this syntax.
Right, good point. If the goal is to provide a workaround then I would be fine if we release the I know this is kind of the same thing but I feel the design of the library API is more friendly if we keep each panic code function and also the custom
Would you agree with this?
Collaborator
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. The linter rules are not ok with that, and I think we should follow them:
I'm not sure that is a good point. When testing panic code with hardhat you do
you don't to
Collaborator
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. I think the current version is simple and effective. It's targetting advanced users anyway. 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.
Fair
We should document the function then. Currently it allows panicking with anything it I think it needs some usage instructions in both the contracts NatSpec and the |
||
/// @solidity memory-safe-assembly | ||
assembly { | ||
mstore(0x00, shl(0xe0, 0x4e487b71)) | ||
mstore(0x04, code) | ||
revert(0x00, 0x24) | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,15 +3,13 @@ | |
|
||
pragma solidity ^0.8.20; | ||
|
||
import {Address} from "../Address.sol"; | ||
import {Panic} from "../Panic.sol"; | ||
|
||
/** | ||
* @dev Standard math utilities missing in the Solidity language. | ||
*/ | ||
library Math { | ||
/** | ||
* @dev Muldiv operation overflow. | ||
*/ | ||
error MathOverflowedMulDiv(); | ||
|
||
enum Rounding { | ||
Floor, // Toward negative infinity | ||
Ceil, // Toward positive infinity | ||
|
@@ -107,7 +105,7 @@ library Math { | |
function ceilDiv(uint256 a, uint256 b) internal pure returns (uint256) { | ||
if (b == 0) { | ||
// Guarantee the same behavior as in a regular Solidity division. | ||
return a / b; | ||
Panic.panic(Panic.DIVISION_BY_ZERO); | ||
} | ||
|
||
// The following calculation ensures accurate ceiling division without overflow. | ||
|
@@ -149,7 +147,7 @@ library Math { | |
|
||
// Make sure the result is less than 2^256. Also prevents denominator == 0. | ||
if (denominator <= prod1) { | ||
revert MathOverflowedMulDiv(); | ||
Panic.panic(denominator == 0 ? Panic.DIVISION_BY_ZERO : Panic.UNDER_OVERFLOW); | ||
} | ||
|
||
/////////////////////////////////////////////// | ||
|
@@ -226,6 +224,9 @@ library Math { | |
* If n is not a prime, then Z/nZ is not a field, and some elements might not be inversible. | ||
* | ||
* If the input value is not inversible, 0 is returned. | ||
* | ||
* NOTE: If you know for sure that n is (big) a prime, it may be cheaper to use Ferma's little theorem and get the | ||
* inverse using `Math.modExp(a, n - 2, n)`. | ||
*/ | ||
function invMod(uint256 a, uint256 n) internal pure returns (uint256) { | ||
unchecked { | ||
|
@@ -275,6 +276,66 @@ library Math { | |
} | ||
} | ||
|
||
/** | ||
* @dev Returns the modular exponentiation of the specified base, exponent and modulus (b ** e % m) | ||
* | ||
* Requirements: | ||
* - modulus can't be zero | ||
* - underlying staticcall to precompile must succeed | ||
* | ||
* IMPORTANT: The result is only valid if the underlying call succeeds. When using this function, make | ||
* sure the chain you're using it on supports the precompiled contract for modular exponentiation | ||
* at address 0x05 as specified in https://eips.ethereum.org/EIPS/eip-198[EIP-198]. Otherwise, | ||
* the underlying function will succeed given the lack of a revert, but the result may be incorrectly | ||
* interpreted as 0. | ||
*/ | ||
function modExp(uint256 b, uint256 e, uint256 m) internal view returns (uint256) { | ||
(bool success, uint256 result) = tryModExp(b, e, m); | ||
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. A potential threat here is that in chains where this precompile is not available, then the result length will be 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. |
||
if (!success) { | ||
if (m == 0) { | ||
Panic.panic(Panic.DIVISION_BY_ZERO); | ||
} else { | ||
revert Address.FailedInnerCall(); | ||
} | ||
} | ||
return result; | ||
} | ||
|
||
/** | ||
* @dev Returns the modular exponentiation of the specified base, exponent and modulus (b ** e % m). | ||
* It includes a success flag indicating if the operation succeeded. Operation will be marked has failed if trying | ||
* to operate modulo 0 or if the underlying precompile reverted. | ||
ernestognw marked this conversation as resolved.
Show resolved
Hide resolved
|
||
* | ||
* IMPORTANT: The result is only valid if the success flag is true. When using this function, make sure the chain | ||
* you're using it on supports the precompiled contract for modular exponentiation at address 0x05 as specified in | ||
* https://eips.ethereum.org/EIPS/eip-198[EIP-198]. Otherwise, the underlying function will succeed given the lack | ||
* of a revert, but the result may be incorrectly interpreted as 0. | ||
*/ | ||
function tryModExp(uint256 b, uint256 e, uint256 m) internal view returns (bool success, uint256 result) { | ||
if (m == 0) return (false, 0); | ||
/// @solidity memory-safe-assembly | ||
assembly { | ||
let ptr := mload(0x40) | ||
// | Offset | Content | Content (Hex) | | ||
// |-----------|------------|--------------------------------------------------------------------| | ||
// | 0x00:0x1f | size of b | 0x0000000000000000000000000000000000000000000000000000000000000020 | | ||
// | 0x20:0x3f | size of e | 0x0000000000000000000000000000000000000000000000000000000000000020 | | ||
// | 0x40:0x5f | size of m | 0x0000000000000000000000000000000000000000000000000000000000000020 | | ||
// | 0x60:0x7f | value of b | 0x<.............................................................b> | | ||
// | 0x80:0x9f | value of e | 0x<.............................................................e> | | ||
// | 0xa0:0xbf | value of m | 0x<.............................................................m> | | ||
mstore(ptr, 0x20) | ||
mstore(add(ptr, 0x20), 0x20) | ||
mstore(add(ptr, 0x40), 0x20) | ||
mstore(add(ptr, 0x60), b) | ||
mstore(add(ptr, 0x80), e) | ||
mstore(add(ptr, 0xa0), m) | ||
|
||
success := staticcall(gas(), 0x05, ptr, 0xc0, 0x00, 0x20) | ||
result := mload(0x00) | ||
} | ||
} | ||
|
||
/** | ||
* @dev Returns the square root of a number. If the number is not a perfect square, the value is rounded | ||
* towards zero. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
const { ethers } = require('hardhat'); | ||
const { expect } = require('chai'); | ||
const { loadFixture } = require('@nomicfoundation/hardhat-network-helpers'); | ||
const { PANIC_CODES } = require('@nomicfoundation/hardhat-chai-matchers/panic'); | ||
|
||
async function fixture() { | ||
return { mock: await ethers.deployContract('$Panic') }; | ||
} | ||
|
||
describe('Panic', function () { | ||
beforeEach(async function () { | ||
Object.assign(this, await loadFixture(fixture)); | ||
}); | ||
|
||
for (const [name, code] of Object.entries({ | ||
GENERIC: 0x0, | ||
ASSERT: PANIC_CODES.ASSERTION_ERROR, | ||
UNDER_OVERFLOW: PANIC_CODES.ARITHMETIC_UNDER_OR_OVERFLOW, | ||
DIVISION_BY_ZERO: PANIC_CODES.DIVISION_BY_ZERO, | ||
ENUM_CONVERSION_ERROR: PANIC_CODES.ENUM_CONVERSION_OUT_OF_BOUNDS, | ||
STORAGE_ENCODING_ERROR: PANIC_CODES.INCORRECTLY_ENCODED_STORAGE_BYTE_ARRAY, | ||
EMPTY_ARRAY_POP: PANIC_CODES.POP_ON_EMPTY_ARRAY, | ||
ARRAY_OUT_OF_BOUNDS: PANIC_CODES.ARRAY_ACCESS_OUT_OF_BOUNDS, | ||
RESOURCE_ERROR: PANIC_CODES.TOO_MUCH_MEMORY_ALLOCATED, | ||
INVALID_INTERNAL_FUNCTION: PANIC_CODES.ZERO_INITIALIZED_VARIABLE, | ||
})) { | ||
describe(`${name} (${ethers.toBeHex(code)})`, function () { | ||
it('exposes panic code as constant', async function () { | ||
expect(await this.mock.getFunction(`$${name}`)()).to.equal(code); | ||
}); | ||
|
||
it('reverts with panic when called', async function () { | ||
await expect(this.mock.$panic(code)).to.be.revertedWithPanic(code); | ||
}); | ||
}); | ||
} | ||
}); |
Uh oh!
There was an error while loading. Please reload this page.