Skip to content

Commit

Permalink
enable eip-1271 for swap, swapAnySender
Browse files Browse the repository at this point in the history
  • Loading branch information
@dmosites
dmosites committed Jan 17, 2024
1 parent efa6912 commit 2e34b4c
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 105 deletions.
110 changes: 47 additions & 63 deletions source/swap-erc20/contracts/SwapERC20.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ pragma solidity 0.8.23;

import "@openzeppelin/contracts/access/Ownable.sol";
import "@openzeppelin/contracts/utils/cryptography/EIP712.sol";
import "@openzeppelin/contracts/utils/cryptography/SignatureChecker.sol";
import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
import "./interfaces/INoReturnERC20.sol";
import "./interfaces/ISwapERC20.sol";
Expand Down Expand Up @@ -256,15 +257,15 @@ contract SwapERC20 is ISwapERC20, Ownable, EIP712 {
);

// Ensure the signatory is not null
if (signatory == address(0)) revert SignatureInvalid();
if (signatory == address(0)) revert Unauthorized();

// Ensure the nonce is not yet used and if not mark it used
if (!_markNonceAsUsed(signatory, nonce)) revert NonceAlreadyUsed(nonce);

// Ensure signatory is authorized to sign
if (authorized[signerWallet] != address(0)) {
// If one is set by signer wallet, signatory must be authorized
if (signatory != authorized[signerWallet]) revert SignatoryUnauthorized();
if (signatory != authorized[signerWallet]) revert Unauthorized();
} else {
// Otherwise, signatory must be signer wallet
if (signatory != signerWallet) revert Unauthorized();
Expand Down Expand Up @@ -447,42 +448,32 @@ contract SwapERC20 is ISwapERC20, Ownable, EIP712 {
order.s = s;
order.senderWallet = senderWallet;

address signatory = ecrecover(
_getOrderHash(
order.nonce,
order.expiry,
order.signerWallet,
order.signerToken,
order.signerAmount,
order.senderWallet,
order.senderToken,
order.senderAmount
),
order.v,
order.r,
order.s
);
address signatory = order.signerWallet;
if (authorized[signatory] != address(0)) {
signatory = authorized[signatory];
}

if (signatory == address(0)) {
errors[errCount] = "SignatureInvalid";
if (
!SignatureChecker.isValidSignatureNow(
signatory,
_getOrderHash(
order.nonce,
order.expiry,
order.signerWallet,
order.signerToken,
order.signerAmount,
order.senderWallet,
order.senderToken,
order.senderAmount
),
abi.encodePacked(r, s, v)
)
) {
errors[errCount] = "Unauthorized";
errCount++;
} else if (nonceUsed(signatory, order.nonce)) {
errors[errCount] = "NonceAlreadyUsed";
errCount++;
} else {
if (
authorized[order.signerWallet] != address(0) &&
signatory != authorized[order.signerWallet]
) {
errors[errCount] = "SignatoryUnauthorized";
errCount++;
} else if (
authorized[order.signerWallet] == address(0) &&
signatory != order.signerWallet
) {
errors[errCount] = "Unauthorized";
errCount++;
} else if (nonceUsed(signatory, order.nonce)) {
errors[errCount] = "NonceAlreadyUsed";
errCount++;
}
}

if (order.expiry < block.timestamp) {
Expand Down Expand Up @@ -639,35 +630,28 @@ contract SwapERC20 is ISwapERC20, Ownable, EIP712 {
// Ensure the expiry is not passed
if (expiry <= block.timestamp) revert OrderExpired();

// Recover the signatory from the hash and signature
(address signatory, ) = ECDSA.tryRecover(
_getOrderHash(
nonce,
expiry,
signerWallet,
signerToken,
signerAmount,
senderWallet,
senderToken,
senderAmount
),
v,
r,
s
);

// Ensure the signatory is not null
if (signatory == address(0)) revert SignatureInvalid();

// Ensure signatory is authorized to sign
if (authorized[signerWallet] != address(0)) {
// If one is set by signer wallet, signatory must be authorized
if (signatory != authorized[signerWallet]) revert SignatoryUnauthorized();
} else {
// Otherwise, signatory must be signer wallet
if (signatory != signerWallet) revert Unauthorized();
address signatory = signerWallet;
if (authorized[signatory] != address(0)) {
signatory = authorized[signatory];
}

if (
!SignatureChecker.isValidSignatureNow(
signatory,
_getOrderHash(
nonce,
expiry,
signerWallet,
signerToken,
signerAmount,
senderWallet,
senderToken,
senderAmount
),
abi.encodePacked(r, s, v)
)
) revert Unauthorized();

// Ensure the nonce is not yet used and if not mark it used
if (!_markNonceAsUsed(signatory, nonce)) revert NonceAlreadyUsed(nonce);
}
Expand Down
2 changes: 0 additions & 2 deletions source/swap-erc20/contracts/interfaces/ISwapERC20.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,7 @@ interface ISwapERC20 {
error MaxTooHigh();
error NonceAlreadyUsed(uint256);
error ScaleTooHigh();
error SignatureInvalid();
error SignatoryInvalid();
error SignatoryUnauthorized();
error Unauthorized();

function swap(
Expand Down
44 changes: 4 additions & 40 deletions source/swap-erc20/test/SwapERC20.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ describe('SwapERC20 Unit', () => {

await expect(
swap.connect(sender).swap(sender.address, ...order)
).to.be.revertedWith('SignatoryUnauthorized')
).to.be.revertedWith('Unauthorized')
})

it('test when signer not authorized', async () => {
Expand Down Expand Up @@ -427,15 +427,6 @@ describe('SwapERC20 Unit', () => {
.to.be.revertedWith('NonceAlreadyUsed')
.withArgs(1)
})

it('test invalid signature', async () => {
const order = await createSignedOrderERC20({}, signer)
order[7] = '29' // Change "v" of signature
await expect(
swap.connect(sender).swap(sender.address, ...order)
).to.be.revertedWith('SignatureInvalid')
})

it('test when signer is zero address', async () => {
const order = await createSignedOrderERC20(
{
Expand Down Expand Up @@ -532,7 +523,7 @@ describe('SwapERC20 Unit', () => {
order[7] = '29' // Change "v" of signature
await expect(
swap.connect(sender).swapAnySender(sender.address, ...order)
).to.be.revertedWith('SignatureInvalid')
).to.be.revertedWith('Unauthorized')
})
})

Expand Down Expand Up @@ -583,7 +574,7 @@ describe('SwapERC20 Unit', () => {
.withArgs(signer.address, anyone.address)

await expect(swap.connect(sender).swapLight(...order)).to.be.revertedWith(
'SignatoryUnauthorized'
'Unauthorized'
)
})
it('test when expiration has passed', async () => {
Expand All @@ -603,7 +594,7 @@ describe('SwapERC20 Unit', () => {
const order = await createSignedOrderERC20({}, signer)
order[7] = '29' // Change "v" of signature
await expect(swap.connect(sender).swapLight(...order)).to.be.revertedWith(
'SignatureInvalid'
'Unauthorized'
)
})
it('test when nonce has already been used', async () => {
Expand Down Expand Up @@ -766,17 +757,6 @@ describe('SwapERC20 Unit', () => {
})

describe('Test check helper', () => {
it('properly detects an invalid signature', async () => {
await setUpAllowances(DEFAULT_AMOUNT, DEFAULT_AMOUNT + SWAP_FEE)
await setUpBalances(DEFAULT_BALANCE, DEFAULT_BALANCE)
const order = await createSignedOrderERC20({}, signer)
order[7] = '29'
const [errCount, messages] = await getErrorInfo(order)
expect(errCount).to.equal(1)
expect(ethers.utils.parseBytes32String(messages[0])).to.equal(
'SignatureInvalid'
)
})
it('properly detects an expired order', async () => {
await setUpAllowances(DEFAULT_AMOUNT, DEFAULT_AMOUNT + SWAP_FEE)
await setUpBalances(DEFAULT_BALANCE, DEFAULT_BALANCE)
Expand All @@ -792,22 +772,6 @@ describe('SwapERC20 Unit', () => {
'OrderExpired'
)
})
it('properly detects a SignatoryUnauthorized() signature', async () => {
await setUpAllowances(DEFAULT_AMOUNT, DEFAULT_AMOUNT + SWAP_FEE)
await setUpBalances(DEFAULT_BALANCE, DEFAULT_BALANCE)
await swap.connect(signer).authorize(anyone.address)
const order = await createSignedOrderERC20(
{
signer,
},
signer
)
const [errCount, messages] = await getErrorInfo(order)
expect(errCount).to.equal(1)
expect(ethers.utils.parseBytes32String(messages[0])).to.equal(
'SignatoryUnauthorized'
)
})
it('properly detects an Unauthorized() signature', async () => {
await setUpAllowances(DEFAULT_AMOUNT, DEFAULT_AMOUNT + SWAP_FEE)
await setUpBalances(DEFAULT_BALANCE, DEFAULT_BALANCE)
Expand Down

0 comments on commit 2e34b4c

Please sign in to comment.