Skip to content
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

chore: migrate GPv2Signing calldata manipulation tests to Foundry #206

Merged
merged 22 commits into from
Aug 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
d0173f0
chore: migrate GPv2Signing domain separator tests to Foundry
fedgiac Aug 9, 2024
c7d4149
Fix unused import
fedgiac Aug 9, 2024
bc7f235
Remove duplicated import
fedgiac Aug 12, 2024
0721892
chore: migrate GPv2Signing set pre-signature tests to Foundry
fedgiac Aug 12, 2024
56ff598
chore: migrate GPv2Signing recoverOrderFromTrade tests to Foundry
fedgiac Aug 12, 2024
f71e28d
Merge branch 'main' into migrate-test-signing-domain-separator
fedgiac Aug 12, 2024
2816827
Add explanatory comment on domain separator struct
fedgiac Aug 12, 2024
585703d
Reuse existing PRE_SIGNED variable from library
fedgiac Aug 12, 2024
d51fced
Merge branch 'main' into migrate-test-signing-domain-separator
fedgiac Aug 12, 2024
e233612
Merge branch 'migrate-test-signing-domain-separator' into migrate-tes…
fedgiac Aug 12, 2024
84eb7fc
Fix missing setting of pre-signature at start of test
fedgiac Aug 12, 2024
6e618dc
Clean up imports
fedgiac Aug 14, 2024
a8c8b45
chore: migrate GPv2Signing calldata manipulation tests to Foundry
fedgiac Aug 14, 2024
483e8f2
Fix fuzz test case where buy and sell tokens are the same
fedgiac Aug 14, 2024
61a0da9
Merge branch 'main' into migrate-test-signing-set-pre-signature
fedgiac Aug 15, 2024
a16ab67
Merge branch 'main' into migrate-test-signing-set-pre-signature
fedgiac Aug 15, 2024
2ebcc44
Merge branch 'migrate-test-signing-set-pre-signature' into migrate-te…
fedgiac Aug 15, 2024
8dd66e7
Merge fuzzed order library into order library
fedgiac Aug 15, 2024
21e12c1
Merge branch 'migrate-test-signing-recover-order-from-trade' into mig…
fedgiac Aug 15, 2024
46fcb81
Update fuzz library usage
fedgiac Aug 15, 2024
cc09214
Merge branch 'main' into migrate-test-signing-recover-order-from-trade
fedgiac Aug 15, 2024
a34bc31
Merge branch 'migrate-test-signing-recover-order-from-trade' into mig…
fedgiac Aug 15, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 1 addition & 71 deletions test/GPv2Signing.test.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import { expect } from "chai";
import { BigNumber, Contract } from "ethers";
import { Contract } from "ethers";
import { artifacts, ethers, waffle } from "hardhat";

import {
EIP1271_MAGICVALUE,
SettlementEncoder,
SigningScheme,
TypedDataDomain,
computeOrderUid,
Expand Down Expand Up @@ -34,75 +33,6 @@ describe("GPv2Signing", () => {
testDomain = domain(chainId, signing.address);
});

describe("recoverOrderFromTrade", () => {
describe("uid uniqueness", () => {
it("invalid EVM transaction encoding does not change order hash", async () => {
// The variables for an EVM transaction are encoded in multiples of 32
// bytes for all types except `string` and `bytes`. This extra padding
// is usually filled with zeroes by the library that creates the
// transaction. It can however be manually messed with, still producing
// a valid transaction.
// Computing GPv2's orderUid requires copying 32-byte-encoded addresses
// from calldata to memory (buy and sell tokens), which are then hashed
// together with the rest of the order. This copying procedure may keep
// the padding bytes as they are in the (manipulated) calldata, since
// Solidity does not make any guarantees on the padding bits of a
// variable during execution. If these 12 padding bits were not zero
// after copying, then the same order would end up with two different
// uids. This test shows that this is not the case.

const encoder = new SettlementEncoder(testDomain);
await encoder.signEncodeTrade(
SAMPLE_ORDER,
traders[0],
SigningScheme.EIP712,
);

const { uid: orderUid } = await signing.recoverOrderFromTradeTest(
encoder.tokens,
encoder.trades[0],
);

const encodedTransactionData = ethers.utils.arrayify(
signing.interface.encodeFunctionData("recoverOrderFromTradeTest", [
encoder.tokens,
encoder.trades[0],
]),
);

// calldata encoding:
// - 4 bytes: signature
// - 32 bytes: pointer to first input value
// - 32 bytes: pointer to second input value
// - 32 bytes: first input value, array -> token array length
// - 32 bytes: first token address
const encodedNumTokens = BigNumber.from(
encodedTransactionData.slice(4 + 2 * 32, 4 + 3 * 32),
);
expect(encodedNumTokens).to.equal(2);
const startTokenWord = 4 + 3 * 32;
const encodedFirstToken = encodedTransactionData.slice(
startTokenWord,
startTokenWord + 32,
);
expect(encodedFirstToken.slice(0, 12).every((byte) => byte === 0)).to.be
.true;
expect(ethers.utils.hexlify(encodedFirstToken.slice(-20))).to.equal(
encoder.tokens[0],
);

for (let i = startTokenWord; i < startTokenWord + 12; i++) {
encodedTransactionData[i] = 42;
}
const encodedOutput = await ethers.provider.call({
data: ethers.utils.hexlify(encodedTransactionData),
to: signing.address,
});
expect(encodedOutput).to.contain(orderUid.slice(2));
});
});
});

describe("recoverOrderSigner", () => {
it("should recover signing address for all supported ECDSA-based schemes", async () => {
for (const scheme of [
Expand Down
82 changes: 82 additions & 0 deletions test/GPv2Signing/CalldataManipulation.t.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
// SPDX-License-Identifier: LGPL-3.0-or-later
pragma solidity ^0.8;

import {Vm} from "forge-std/Test.sol";

import {GPv2Order, GPv2Signing, IERC20} from "src/contracts/mixins/GPv2Signing.sol";

import {GPv2SigningTestInterface, Helper} from "./Helper.sol";

import {Bytes} from "test/libraries/Bytes.sol";
import {Order} from "test/libraries/Order.sol";
import {SettlementEncoder} from "test/libraries/encoders/SettlementEncoder.sol";

contract CalldataManipulation is Helper {
using SettlementEncoder for SettlementEncoder.State;
using Bytes for bytes;

Vm.Wallet private trader;

constructor() {
trader = vm.createWallet("GPv2Signing.RecoverOrderFromTrade: trader");
}

function test_invalid_EVM_transaction_encoding_does_not_change_order_hash(
Order.Fuzzed memory params,
uint256 paddingIndex
) public {
// The variables for an EVM transaction are encoded in multiples of 32
// bytes for all types except `string` and `bytes`. This extra padding
// is usually filled with zeroes by the library that creates the
// transaction. It can however be manually messed with.
// Since Solidity v0.8, using nonzero padding should cause the
// transaction to revert.
// Computing GPv2's orderUid requires copying 32-byte-encoded addresses
// from calldata to memory (buy and sell tokens), which are then hashed
// together with the rest of the order. This copying procedure would
// keep the padding bytes as they are in the (manipulated) calldata.
// If these 12 padding bits were not zero after copying, then the same
// order would end up with two different uids. This test shows that this
// is not the case by showing that such calldata manipulation causes the
// transaction to revert.

GPv2Order.Data memory order = Order.fuzz(params);

SettlementEncoder.State storage encoder = SettlementEncoder.makeSettlementEncoder();
encoder.signEncodeTrade(vm, trader, order, domainSeparator, GPv2Signing.Scheme.Eip712, 0);

IERC20[] memory tokens = encoder.tokens();
bytes memory encodedTransactionData =
abi.encodeCall(GPv2SigningTestInterface.recoverOrderFromTradeTest, (tokens, encoder.trades[0]));

// calldata encoding:
// - 4 bytes: signature
// - 32 bytes: pointer to first input value
// - 32 bytes: pointer to second input value
// - 32 bytes: first input value, array -> token array length
// - 32 bytes: first token address
uint256 startNumTokenWord = 4 + 2 * 32;
uint256 startFirstTokenWord = startNumTokenWord + 32;
uint256 encodedNumTokens = abi.decode(encodedTransactionData.slice(startNumTokenWord, 32), (uint256));
require(
encodedNumTokens == ((order.sellToken == order.buyToken) ? 1 : 2),
"invalid test setup; has the transaction encoding changed?"
);
bytes memory encodedFirstToken = encodedTransactionData.slice(startFirstTokenWord, 32);
uint256 tokenPaddingSize = 12;
for (uint256 i = 0; i < tokenPaddingSize; i++) {
require(encodedFirstToken[i] == bytes1(0), "invalid test setup; has the transaction encoding changed?");
}
IERC20 token = IERC20(abi.decode(encodedFirstToken, (address)));
require(token == tokens[0], "invalid test setup; has the transaction encoding changed?");
// Here we change a single padding byte, this is enough to make the
// transaction revert.
paddingIndex = paddingIndex % tokenPaddingSize;
encodedTransactionData[startFirstTokenWord + paddingIndex] = 0x42;
// Using low-level call because we want to call this function with data
// that is intentionally invalid.
// solhint-disable-next-line avoid-low-level-calls
(bool success,) = address(executor).call(encodedTransactionData);
assertFalse(success);
}
}
Loading