Conversation
snreynolds
force-pushed
the
1155-cherrypick
branch
from
00ad685 to
5cd3a7c
Compare
| revert AllowanceExpired(allowed.expiration, operator.expiration); | ||
| } | ||
|
|
||
| uint256 maxAmount = allowed.amount; |
There was a problem hiding this comment.
Whats the reason for having this as a 256 for a while and then casting it back to a 160 later?
| if (operatorExpired) revert InsufficientAllowance(maxAmount); | ||
| } else { | ||
| unchecked { | ||
| allowed.amount = uint160(maxAmount) - amount; |
There was a problem hiding this comment.
wait isn't this backwards? like, if approval is MAX then we shouldn't update, but if it's not MAX then we should update?
| } | ||
|
|
||
| uint256 maxAmount = allowed.amount; | ||
| if (maxAmount != type(uint160).max) { |
There was a problem hiding this comment.
could save a level of nesting here i think by switching the ordering around:
if (maxAmount == type(uint160).max) {
... max stuff
} else if (amount > maxAmount) {
revert
}
| } | ||
|
|
||
| /// @inheritdoc IAllowanceTransferERC1155 | ||
| function invalidateNonces(address token, address spender, uint48 newNonce) external { |
There was a problem hiding this comment.
not sure but maybe invalidateOperatorNonces better for clarity?
| /// @notice EIP712 helpers for Permit2 ERC1155s | ||
| /// @dev Maintains cross-chain replay protection in the event of a fork | ||
| /// @dev Reference: https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/utils/cryptography/EIP712.sol | ||
| contract EIP712ForERC1155 { |
There was a problem hiding this comment.
isnt this contract the same for all types minus the _HASHED_NAME? wonder if we could have it be shared and take the name as constructor arg
| /// @return bitPos The bit position | ||
| /// @dev The first 248 bits of the nonce value is the index of the desired bitmap | ||
| /// @dev The last 8 bits of the nonce value is the position of the bit in the bitmap | ||
| function bitmapPositions(uint256 nonce) private pure returns (uint256 wordPos, uint256 bitPos) { |
There was a problem hiding this comment.
could these be in like an UnorderedNonceLibrary since theyre shared across all 3 instances?
| "PermitBatch(PermitDetails[] details,address spender,uint256 sigDeadline)PermitDetails(address token,uint160 amount,uint48 expiration,uint48 nonce)" | ||
| ); | ||
|
|
||
| bytes32 public constant _TOKEN_PERMISSIONS_TYPEHASH = keccak256("TokenPermissions(address token,uint256 amount)"); |
There was a problem hiding this comment.
doesnt this need tokenId?
There was a problem hiding this comment.
also could just be keccak256(TOKEN_PERMISSIONS_TYPESTRING)
| "PermitTransferFrom(TokenPermissions permitted,address spender,uint256 nonce,uint256 deadline)TokenPermissions(address token,uint256 amount)" | ||
| ); | ||
|
|
||
| bytes32 public constant _PERMIT_BATCH_TRANSFER_FROM_TYPEHASH = keccak256( |
There was a problem hiding this comment.
nit: maybe string.concat("PermitBatch....", _TOKEN_PERMISSIONS_TYPEHASH) to help avoid typos?
3 participants
No description provided.