Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
86 changes: 86 additions & 0 deletions examples/kyber/test/KyberMetaAggregationRouterAssertion.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,92 @@ 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 originalRouterFamily_ = false (the backtest-harness misconfiguration).
Comment thread
makemake-kbo marked this conversation as resolved.
Outdated
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 compiler-generated constructor prologue ABI-decodes its
/// arguments from the code-appended tail and reverts when the tail is too short to contain
Comment thread
makemake-kbo marked this conversation as resolved.
Outdated
/// the `bool` — so 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 too-short constructor tail cannot be decoded into (address,bool).
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)
Comment thread
makemake-kbo marked this conversation as resolved.
Outdated
);
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
Loading