Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
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
2 changes: 1 addition & 1 deletion .github/workflows/solidity-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ jobs:
strategy:
fail-fast: false
matrix:
profile: [balancer, fluid, lido, mellow, safe-guard]
profile: [balancer, fluid, kyber, lido, mellow, safe-guard]
Comment thread
makemake-kbo marked this conversation as resolved.
steps:
- name: Checkout
uses: actions/checkout@v6
Expand Down
93 changes: 90 additions & 3 deletions examples/kyber/test/KyberMetaAggregationRouterAssertion.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,8 @@ contract KyberMetaAggregationRouterAssertionTest is Test, CredibleTest {
// ---------------------------------------------------------------

function _arm(bytes4 fnSelector) internal {
bytes memory createData = abi.encodePacked(
type(KyberMetaAggregationRouterAssertion).creationCode, abi.encode(address(router), true)
);
bytes memory createData =
abi.encodePacked(type(KyberMetaAggregationRouterAssertion).creationCode, abi.encode(address(router), true));
cl.assertion(address(router), createData, fnSelector);
}

Expand Down Expand Up @@ -375,6 +374,94 @@ contract KyberMetaAggregationRouterAssertionTest is Test, CredibleTest {
assertLt(dst.balanceOf(recipient), fullMinimum);
}

/// @notice The same legitimate partial fill FALSE-POSITIVES when the assertion is deployed with
/// an explicit `originalRouterFamily_ = false`, isolating the behavioural consequence of a
/// wrong family flag.
/// @dev `_arm` uses the correct `true`; here we deploy with an explicit `false`. With the family
/// flag false the `_PARTIAL_FILL` skip short-circuits off, so the flat `>= minReturnAmount`
/// floor rejects a swap the router legitimately filled pro-rata. The assertion logic is
/// unchanged and correct.
///
/// NOTE: this is the *explicit-false* misconfiguration, NOT the mainnet backtest harness's
/// actual bug. The harness encoded a single-`address` constructor tail, which does not read
/// the missing bool as zero — it reverts construction outright
/// (`AssertionContractDeployFailed`). See
/// `testDeployment_OneArgPayload_RevertsDuringConstruction` for the real root cause; this
/// test only demonstrates what a *successfully deployed* wrong-family assertion would do.
function testMinReturn_PartialFill_MisconfiguredFamilyFalse_Trips() public {
Comment thread
makemake-kbo marked this conversation as resolved.
Outdated
uint256 spent = AMOUNT_IN / 2;
uint256 fullMinimum = _expectedOut();

SwapDescriptionV2 memory desc;
desc.srcToken = address(src);
desc.dstToken = address(dst);
desc.srcReceivers = _one(address(pool));
desc.srcAmounts = _one(spent);
desc.feeReceivers = _noAddrs();
desc.feeAmounts = _noUints();
desc.dstReceiver = recipient;
desc.amount = AMOUNT_IN;
desc.minReturnAmount = fullMinimum;
desc.flags = PARTIAL_FILL;
Comment thread
makemake-kbo marked this conversation as resolved.

SwapExecutionParams memory p;
p.callTarget = address(executor);
p.targetData = _executorData(recipient);
p.desc = desc;

// Arm with an explicit originalRouterFamily_ = false: a separate, hypothetical wrong-family
// configuration -- NOT the backtest-harness bug (that was the malformed one-argument
// constructor tail, which fails deployment; see testDeployment_OneArgPayload_RevertsDuringConstruction).
bytes memory createData = abi.encodePacked(
Comment thread
makemake-kbo marked this conversation as resolved.
Outdated
type(KyberMetaAggregationRouterAssertion).creationCode, abi.encode(address(router), false)
);
cl.assertion(
address(router), createData, KyberMetaAggregationRouterAssertion.assertReceiverGetsMinReturn.selector
);

vm.expectRevert(bytes("Kyber: dstReceiver credited below minReturnAmount"));
router.swap(p);
}

/// @notice The REAL one-argument deployment payload `creationCode || abi.encode(address(router))`
/// REVERTS during construction — it does NOT silently zero the trailing bool.
/// @dev This pins the actual root cause of the mainnet backtest trip. The harness built the
/// assertion create data with a single-arg `constructor(address)` ABI encoding (32 bytes of
/// constructor tail), but the assertion's constructor is the 2-arg
/// `constructor(address,bool)`. The generated constructor copies only the appended argument
/// tail (`codesize() - programSize`), so this payload gives the tuple decoder 32 bytes where
/// it requires 64. Decoding reverts on the short tail before reading either value,
/// construction aborts, and `CREATE` returns `address(0)`. The trailing bool never "reads as
/// zero".
///
/// Under `pcl`/`cl.assertion` this same construction revert surfaces as
/// `AssertionContractDeployFailed` before the triggered `router.swap` ever runs (verified on
/// PCL 1.5.1 / Solc 0.8.30 by fredo, and reproduced here on PCL 1.6.0 / Solc 0.8.34). That
/// cheatcode path aborts the whole `pcl test` process rather than raising a catchable
/// Solidity revert, so this regression is pinned deterministically at the construction layer
/// via low-level `CREATE`: the exact one-argument payload fails, the correct two-argument
/// payload succeeds.
function testDeployment_OneArgPayload_RevertsDuringConstruction() public {
Comment thread
makemake-kbo marked this conversation as resolved.
Outdated
// The exact payload the backtest harness built: creation code + a single `address` arg.
bytes memory oneArgPayload =
abi.encodePacked(type(KyberMetaAggregationRouterAssertion).creationCode, abi.encode(address(router)));
address oneArgDeployed;
assembly {
oneArgDeployed := create(0, add(oneArgPayload, 0x20), mload(oneArgPayload))
}
// Construction reverted: the tuple decoder received 32 bytes but requires 64.
assertEq(oneArgDeployed, address(0), "one-arg payload must fail construction, not zero the bool");

// The correct two-argument payload constructs successfully.
bytes memory twoArgPayload =
abi.encodePacked(type(KyberMetaAggregationRouterAssertion).creationCode, abi.encode(address(router), true));
address twoArgDeployed;
assembly {
twoArgDeployed := create(0, add(twoArgPayload, 0x20), mload(twoArgPayload))
}
assertTrue(twoArgDeployed != address(0), "two-arg payload must construct");
}

/// @notice swapSimpleMode underpayment trips when the router guard is absent.
function testMinReturn_SwapSimpleMode_Underpaid_Trips() public {
router.setEnforceMinReturn(false);
Expand Down