-
Notifications
You must be signed in to change notification settings - Fork 24
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
[Feature] Spender permit feature #110
Open
charlesjhongc
wants to merge
60
commits into
master
Choose a base branch
from
spender_permit_feature
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+1,399
−404
Open
Changes from 20 commits
Commits
Show all changes
60 commits
Select commit
Hold shift + click to select a range
d1d0156
feat: spender contract and foundry test
oneleo 93833a5
Merge branch 'master' into LABS-960/rfq-with-spender-permit
oneleo ecbe62e
bug: change to new signature format
oneleo b05f0ec
feat: replace rfq spendfromuser func with spendfromusertowithpermit
oneleo a05cfcd
refac: leverage struct spendwithpermit in params
oneleo eae8717
chg: modify variable names for readability
oneleo d89c922
refac: move require condition from rfq to spender contract
oneleo 89c376a
refac: modify variables and function in rfq foundry test
oneleo 8af0b60
chg: modify variable names for readability
oneleo b5b3793
spec: Add requester verification foundry test
oneleo b2f296b
refac: leverage struct spendwithpermit in params
oneleo 9dca7b1
bug: assign replay protection check before sig valid check
oneleo c376229
bug: use safeTransfer() in SafeERC20 instead
oneleo 2d21eaa
feat: remove taker/maker spendWithPermit but create they from _order
oneleo d493f7c
refac: convert the BPS_MAX variable appropriately
oneleo 31ad782
refac: move taker/maker spendWithPermit from fill() to _settle()
oneleo ac38d3e
feat: modify error message returned when replay protection checks
oneleo 123e7df
refac: use only libconstant.bps_max variable
oneleo c310fa1
chg: run prettier to format spender.sol to resolve code style issues
oneleo 556ddc7
Merge pull request #99 from consenlabs/LABS-940/spender-adds-backward…
oneleo 65d0d60
Merge branch 'master' into modify-salt-usage-of-spendwithpermit
oneleo 9f242b9
feat: spendWithPermit should contain transaction or order hash
oneleo 45e146f
Merge branch 'master' into spender_permit_feature
oneleo 049cda6
Merge branch 'spender_permit_feature' into modify-salt-usage-of-spend…
oneleo 8acfb68
Merge pull request #105 from consenlabs/modify-salt-usage-of-spendwit…
oneleo c8b8f2d
Merge branch 'master' into spender_permit_feature
oneleo 2a5da3d
feat: spendWithPermit should contain transaction or order hash
oneleo 36d2730
Merge pull request #111 from consenlabs/modify-txhash-name-of-spendwi…
charlesjhongc db4726e
refac: rfq, spender contracts changed to non named parameter type
oneleo db1564a
Merge branch 'master' into spender_permit_feature
oneleo 03d5b72
Merge pull request #112 from consenlabs/rfq_spender_to_no_named_param…
oneleo 77fb55a
refac: move the signSpendWithPermit function in foundry test
oneleo 5627a14
perf: use eip712 domain separator instead of spender contract
oneleo b9f4bac
bug: revert when sigType is invalid
oneleo 9a1d452
modify spendFromUser --> spendFromUserToWithPermit
a130849
add helper function regarding spenderPermit on l2Deposit action
f17e7ab
modify test case using spenderPermit
0714df3
replace comment for _createSpenderPermitFromL2Deposit
c3377c7
remove accidentally added files
f02e23a
remove redundant comments
4dd27c9
fix nit
7b4392e
modify solidity objects using named parameters
be48682
modify DEFAULT_DEPOSIT to _deposit in _createSpenderPermitFromL2Deposit
b66e418
Merge pull request #114 from consenlabs/move_signspendwithpermit_func
oneleo 6bdf206
feat: replace amm spendfromuser func with spendfromusertowithpermit
oneleo a52be1c
chg: modify variable name to match internal variable format
oneleo 7150101
Merge branch 'spender_permit_feature' into modify-l2deposit-using-spe…
a0e582e
remove signSpendWithPermit() from Setup.t.sol
4130802
use signSpendWithPermit() from contracts-test/utils/Permit.sol
58d1b78
refec: modify comments and function names
oneleo 4aa11d0
Merge pull request #115 from consenlabs/modify-l2deposit-using-spende…
108356037 eb9ca88
refec: modify comments and remove unused imported package
oneleo 66b606f
doc: adject _transferTakerAssetToAMM func comment to be clearer
oneleo 2619aca
doc: use named parameters to make code clearer
oneleo 6c542db
doc: apply named parameters to transferTakerAssetToAMM in ammwrapper
oneleo 424feec
Merge pull request #116 from consenlabs/amm-with-spender-permit
oneleo a67eba2
refac: handle the conflict between RFQ.t.sol and the master branch
oneleo e8b9284
Merge branch 'master' into spender_permit_feature
oneleo 8dcef47
feat: remove spendFromUser() func in Spender.sol
oneleo d1e7f20
Merge pull request #119 from consenlabs/remove_spendfromuser_function
charlesjhongc File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,39 +1,40 @@ | ||
// SPDX-License-Identifier: MIT | ||
pragma solidity 0.7.6; | ||
pragma abicoder v2; | ||
|
||
import "forge-std/Test.sol"; | ||
import "@openzeppelin/contracts/token/ERC20/IERC20.sol"; | ||
import "@openzeppelin/contracts/token/ERC20/SafeERC20.sol"; | ||
import "contracts/Spender.sol"; | ||
import "contracts/SpenderSimulation.sol"; | ||
import "contracts/AllowanceTarget.sol"; | ||
import "contracts/utils/SpenderLibEIP712.sol"; | ||
import "contracts-test/mocks/MockERC20.sol"; | ||
import "contracts-test/mocks/MockDeflationaryERC20.sol"; | ||
import "contracts-test/mocks/MockNoReturnERC20.sol"; | ||
import "contracts-test/mocks/MockNoRevertERC20.sol"; | ||
import "contracts-test/utils/BalanceUtil.sol"; | ||
import "contracts-test/utils/BalanceSnapshot.sol"; | ||
|
||
contract SpenderTest is BalanceUtil { | ||
using BalanceSnapshot for BalanceSnapshot.Snapshot; | ||
using SafeERC20 for IERC20; | ||
|
||
event TearDownAllowanceTarget(uint256 tearDownTimeStamp); | ||
struct SpendWithPermit { | ||
address tokenAddr; | ||
address user; | ||
address recipient; | ||
uint256 amount; | ||
uint256 salt; | ||
uint64 expiry; | ||
} | ||
|
||
uint256 userPrivateKey = uint256(1); | ||
uint256 otherPrivateKey = uint256(2); | ||
|
||
address user = vm.addr(userPrivateKey); | ||
// Originally requester should be the address of the strategy contract | ||
// that calls spender; But in this test case, the requester is address(this) | ||
address requester = address(this); | ||
address recipient = address(0x133702); | ||
address unauthorized = address(0x133704); | ||
address[] wallet = [address(this), user, recipient]; | ||
|
||
Spender spender; | ||
SpenderSimulation spenderSimulation; | ||
AllowanceTarget allowanceTarget; | ||
MockERC20 lon = new MockERC20("TOKENLON", "LON", 18); | ||
MockDeflationaryERC20 deflationaryERC20 = new MockDeflationaryERC20(); | ||
|
@@ -42,7 +43,7 @@ contract SpenderTest is BalanceUtil { | |
IERC20[] tokens = [IERC20(address(deflationaryERC20)), IERC20(address(noReturnERC20)), IERC20(address(noRevertERC20))]; | ||
|
||
uint64 EXPIRY = uint64(block.timestamp + 1); | ||
SpendWithPermit DEFAULT_SPEND_WITH_PERMIT; | ||
SpenderLibEIP712.SpendWithPermit DEFAULT_SPEND_WITH_PERMIT; | ||
|
||
// effectively a "beforeEach" block | ||
function setUp() public { | ||
|
@@ -55,6 +56,11 @@ contract SpenderTest is BalanceUtil { | |
// Setup | ||
spender.setAllowanceTarget(address(allowanceTarget)); | ||
spender.authorize(wallet); | ||
// Deploy spenderSimulation contract adn set its address to authorization list of spender | ||
spenderSimulation = new SpenderSimulation(spender, new address[](0)); | ||
address[] memory spenderSimulationAddr = new address[](1); | ||
spenderSimulationAddr[0] = address(spenderSimulation); | ||
spender.authorize(spenderSimulationAddr); | ||
|
||
// Deal 100 ETH to each account | ||
for (uint256 i = 0; i < wallet.length; i++) { | ||
|
@@ -74,8 +80,9 @@ contract SpenderTest is BalanceUtil { | |
|
||
// Default SpendWithPermit | ||
// prettier-ignore | ||
DEFAULT_SPEND_WITH_PERMIT = SpendWithPermit( | ||
DEFAULT_SPEND_WITH_PERMIT = SpenderLibEIP712.SpendWithPermit( | ||
address(lon), // tokenAddr | ||
requester, // requester | ||
user, // user | ||
recipient, // receipient | ||
100 * 1e18, // amount | ||
|
@@ -84,11 +91,13 @@ contract SpenderTest is BalanceUtil { | |
); | ||
|
||
// Label addresses for easier debugging | ||
vm.label(requester, "Requester"); | ||
vm.label(user, "User"); | ||
vm.label(recipient, "Recipient"); | ||
vm.label(unauthorized, "Unauthorized"); | ||
vm.label(address(this), "TestingContract"); | ||
vm.label(address(spender), "SpenderContract"); | ||
vm.label(address(spenderSimulation), "SpenderSimulationContract"); | ||
vm.label(address(allowanceTarget), "AllowanceTargetContract"); | ||
vm.label(address(lon), "LON"); | ||
} | ||
|
@@ -306,4 +315,159 @@ contract SpenderTest is BalanceUtil { | |
|
||
assertEq(noReturnERC20.balanceOf(recipient), 100); | ||
} | ||
|
||
/******************************************* | ||
* Test: spendFromUserToWithPermit * | ||
*******************************************/ | ||
|
||
function testCannotSpendFromUserToWithPermitWithExpiredPermit() public { | ||
SpenderLibEIP712.SpendWithPermit memory spendWithPermit = DEFAULT_SPEND_WITH_PERMIT; | ||
spendWithPermit.expiry = uint64(block.timestamp - 1); // Timestamp expired | ||
bytes memory sig = _signSpendWithPermit(userPrivateKey, spendWithPermit); | ||
|
||
vm.expectRevert("Spender: Permit is expired"); | ||
spender.spendFromUserToWithPermit(spendWithPermit, sig); | ||
} | ||
|
||
function testCannotSpendFromUserToWithPermitWithWrongRequester() public { | ||
SpenderLibEIP712.SpendWithPermit memory spendWithPermit = DEFAULT_SPEND_WITH_PERMIT; | ||
spendWithPermit.requester = unauthorized; // Wrong requester address | ||
bytes memory sig = _signSpendWithPermit(userPrivateKey, spendWithPermit); | ||
|
||
vm.expectRevert("Spender: invalid requester address"); | ||
spender.spendFromUserToWithPermit(spendWithPermit, sig); | ||
} | ||
|
||
function testCannotSpendFromUserToWithPermitByWrongSigner() public { | ||
SpenderLibEIP712.SpendWithPermit memory spendWithPermit = DEFAULT_SPEND_WITH_PERMIT; | ||
bytes memory sig = _signSpendWithPermit(otherPrivateKey, spendWithPermit); // Wrong signer | ||
|
||
vm.expectRevert("Spender: Invalid permit signature"); | ||
spender.spendFromUserToWithPermit(spendWithPermit, sig); | ||
} | ||
|
||
function testCannotSpendFromUserToWithPermitWithWrongRecipient() public { | ||
SpenderLibEIP712.SpendWithPermit memory spendWithPermit = DEFAULT_SPEND_WITH_PERMIT; | ||
bytes memory sig = _signSpendWithPermit(userPrivateKey, spendWithPermit); | ||
|
||
vm.expectRevert("Spender: Invalid permit signature"); | ||
spendWithPermit.recipient = unauthorized; // recipient is different from signed | ||
spender.spendFromUserToWithPermit(spendWithPermit, sig); | ||
} | ||
|
||
function testCannotSpendFromUserToWithPermitWithWrongToken() public { | ||
SpenderLibEIP712.SpendWithPermit memory spendWithPermit = DEFAULT_SPEND_WITH_PERMIT; | ||
bytes memory sig = _signSpendWithPermit(userPrivateKey, spendWithPermit); | ||
|
||
vm.expectRevert("Spender: Invalid permit signature"); | ||
spendWithPermit.tokenAddr = unauthorized; // tokenAddr is different from signed | ||
spender.spendFromUserToWithPermit(spendWithPermit, sig); | ||
} | ||
|
||
function testCannotSpendFromUserToWithPermitWithWrongAmount() public { | ||
SpenderLibEIP712.SpendWithPermit memory spendWithPermit = DEFAULT_SPEND_WITH_PERMIT; | ||
bytes memory sig = _signSpendWithPermit(userPrivateKey, spendWithPermit); | ||
|
||
vm.expectRevert("Spender: Invalid permit signature"); | ||
spendWithPermit.amount = spendWithPermit.amount + 1; // amount is different from signed | ||
spender.spendFromUserToWithPermit(spendWithPermit, sig); | ||
} | ||
|
||
function testCannotSpendFromUserToWithPermitByNotAuthorized() public { | ||
SpenderLibEIP712.SpendWithPermit memory spendWithPermit = DEFAULT_SPEND_WITH_PERMIT; | ||
bytes memory sig = _signSpendWithPermit(userPrivateKey, spendWithPermit); | ||
|
||
vm.expectRevert("Spender: not authorized"); | ||
vm.prank(unauthorized); // Only authorized strategy contracts and owner | ||
spender.spendFromUserToWithPermit(spendWithPermit, sig); | ||
} | ||
|
||
function testCannotSpendFromUserToWithPermitWithBlacklistedToken() public { | ||
address[] memory blacklistAddress = new address[](1); | ||
blacklistAddress[0] = address(lon); | ||
bool[] memory blacklistBool = new bool[](1); | ||
blacklistBool[0] = true; | ||
spender.blacklist(blacklistAddress, blacklistBool); // Set lon to black list by owner (this contract) | ||
SpenderLibEIP712.SpendWithPermit memory spendWithPermit = DEFAULT_SPEND_WITH_PERMIT; | ||
bytes memory sig = _signSpendWithPermit(userPrivateKey, spendWithPermit); | ||
|
||
vm.expectRevert("Spender: token is blacklisted"); | ||
spender.spendFromUserToWithPermit(spendWithPermit, sig); | ||
} | ||
|
||
function testCannotSpendFromUserToWithPermitWithSamePermit() public { | ||
SpenderLibEIP712.SpendWithPermit memory spendWithPermit = DEFAULT_SPEND_WITH_PERMIT; | ||
bytes memory sig = _signSpendWithPermit(userPrivateKey, spendWithPermit); | ||
spender.spendFromUserToWithPermit(spendWithPermit, sig); | ||
|
||
vm.expectRevert("Spender: Permit is already fulfilled"); | ||
spender.spendFromUserToWithPermit(spendWithPermit, sig); // Detected the same permit hash in the past | ||
} | ||
|
||
function testSpendFromUserToWithPermit() public { | ||
BalanceSnapshot.Snapshot memory recipientLon = BalanceSnapshot.take(recipient, address(lon)); | ||
SpenderLibEIP712.SpendWithPermit memory spendWithPermit = DEFAULT_SPEND_WITH_PERMIT; | ||
bytes memory sig = _signSpendWithPermit(userPrivateKey, spendWithPermit); | ||
spender.spendFromUserToWithPermit(spendWithPermit, sig); | ||
|
||
recipientLon.assertChange(int256(spendWithPermit.amount)); // Confirm amount of tokens received | ||
} | ||
|
||
/******************************************* | ||
* Test: simulate * | ||
*******************************************/ | ||
|
||
function testCannotSimulateByNotAuthorized() public { | ||
SpenderLibEIP712.SpendWithPermit memory spendWithPermit = DEFAULT_SPEND_WITH_PERMIT; | ||
// Set the requester address to spenderSimulation contract | ||
spendWithPermit.requester = address(spenderSimulation); | ||
bytes memory sig = _signSpendWithPermit(userPrivateKey, spendWithPermit); | ||
// Set address of spenderSimulation to be removed from authorization list of spender, | ||
// and spenderSimulation can not execute spender.spendFromUserToWithPermit() function. | ||
address[] memory spenderSimulationAddr = new address[](1); | ||
spenderSimulationAddr[0] = address(spenderSimulation); | ||
spender.deauthorize(spenderSimulationAddr); | ||
|
||
vm.expectRevert("Spender: not authorized"); | ||
spenderSimulation.simulate(spendWithPermit, sig); | ||
} | ||
|
||
function testSimulate() public { | ||
SpenderLibEIP712.SpendWithPermit memory spendWithPermit = DEFAULT_SPEND_WITH_PERMIT; | ||
// Set the requester address to spenderSimulation contract | ||
spendWithPermit.requester = address(spenderSimulation); | ||
bytes memory sig = _signSpendWithPermit(userPrivateKey, spendWithPermit); | ||
|
||
vm.expectRevert("SpenderSimulation: transfer simulation success"); | ||
spenderSimulation.simulate(spendWithPermit, sig); | ||
} | ||
|
||
/********************************* | ||
* Helpers * | ||
*********************************/ | ||
|
||
function _getEIP712Hash(bytes32 structHash) internal view returns (bytes32) { | ||
string memory EIP191_HEADER = "\x19\x01"; | ||
bytes32 EIP712_DOMAIN_SEPARATOR = spender.EIP712_DOMAIN_SEPARATOR(); | ||
return keccak256(abi.encodePacked(EIP191_HEADER, EIP712_DOMAIN_SEPARATOR, structHash)); | ||
} | ||
|
||
function _signSpendWithPermit(uint256 privateKey, SpenderLibEIP712.SpendWithPermit memory spendWithPermit) internal returns (bytes memory sig) { | ||
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. Remove. |
||
uint256 SPEND_WITH_PERMIT_TYPEHASH = 0xab1af22032364b17f69bad7eabde29f0cd3f761861c0343407be7fcac2e3ff1f; | ||
bytes32 structHash = keccak256( | ||
abi.encode( | ||
SPEND_WITH_PERMIT_TYPEHASH, | ||
spendWithPermit.tokenAddr, | ||
spendWithPermit.requester, | ||
spendWithPermit.user, | ||
spendWithPermit.recipient, | ||
spendWithPermit.amount, | ||
spendWithPermit.salt, | ||
spendWithPermit.expiry | ||
) | ||
); | ||
bytes32 spendWithPermitHash = _getEIP712Hash(structHash); | ||
(uint8 v, bytes32 r, bytes32 s) = vm.sign(privateKey, spendWithPermitHash); | ||
sig = abi.encodePacked(r, s, v, uint8(2)); // SignatureType = 2 = EIP712 | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
// SPDX-License-Identifier: MIT | ||
pragma solidity ^0.7.6; | ||
|
||
library SpenderLibEIP712 { | ||
struct SpendWithPermit { | ||
address tokenAddr; | ||
address requester; | ||
address user; | ||
address recipient; | ||
uint256 amount; | ||
uint256 salt; | ||
uint64 expiry; | ||
} | ||
/* | ||
keccak256( | ||
abi.encodePacked( | ||
"SpendWithPermit(", | ||
"address tokenAddr,", | ||
"address requester,", | ||
"address user,", | ||
"address recipient,", | ||
"uint256 amount,", | ||
"uint256 salt,", | ||
"uint64 expiry", | ||
")" | ||
) | ||
); | ||
*/ | ||
uint256 public constant SPEND_WITH_PERMIT_TYPEHASH = 0xab1af22032364b17f69bad7eabde29f0cd3f761861c0343407be7fcac2e3ff1f; | ||
|
||
function _getSpendWithPermitHash(SpendWithPermit memory _spendWithPermit) internal pure returns (bytes32) { | ||
return | ||
keccak256( | ||
abi.encode( | ||
SPEND_WITH_PERMIT_TYPEHASH, | ||
_spendWithPermit.tokenAddr, | ||
_spendWithPermit.requester, | ||
_spendWithPermit.user, | ||
_spendWithPermit.recipient, | ||
_spendWithPermit.amount, | ||
_spendWithPermit.salt, | ||
_spendWithPermit.expiry | ||
) | ||
); | ||
} | ||
} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Reused the one in
contracts-test/utils/Sig.sol
.