fix(kyber): bind recipient-credit checks to router-family semantics

.github/workflows/solidity-test.yml

@@ -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]
     steps:
       - name: Checkout
         uses: actions/checkout@v6

examples/kyber/BACKTEST_TRIP_FINDINGS.md

@@ -0,0 +1,102 @@
+# Kyber assertion — mainnet backtest trip (RESOLVED)
+
+Durable record of the ENG-3860 seven-day replay backtest of
+`KyberMetaAggregationRouterAssertion` and its triage outcome. This file is intentionally
+sanitized: it names the replay service and its archive requirement but contains no access
+tokens, and it pins toolchain-dependent artifacts (assertion IDs) to a reproducible command
+rather than hard-coding stale hashes.
+
+## Summary
+
+| Field | Value |
+| --- | --- |
+| Chain | 1 (Ethereum mainnet) |
+| Adopter | `0x6131B5fae19EA4f9D964eAc0408E4408b66337b5` (KyberSwap MetaAggregationRouterV2, live on mainnet) |
+| Assertion | `examples/kyber/src/KyberMetaAggregationRouterAssertion.sol` (`assertReceiverGetsMinReturn`, triggered on `swap`/`swapGeneric`/`swapSimpleMode`) |
+| Requested window | `start_block = 25480001` → `head = 25487144` (7143 blocks, ~7 days), frozen |
+| Scan outcome | Early `complete` — stopped at `current_block = 25480033` (« `destination_block`), i.e. `ReplayStopMatch` |
+| Reverts | ≥ 1 — the matched transaction that stopped the scan (poll-only run; see note) |
+| Triage outcome | **Test-harness deployment bug, not an assertion bug. No assertion code change warranted.** |
+
+Note on counts: the job was submitted without a `callback_url`, so only
+`GET /backtest/{job}/progress` was observable. That endpoint surfaces the stop block, not a
+per-transaction tally, so the recorded revert figure is the single trip that stopped the scan
+rather than a full tx/revert census. This poll-only limitation is a known property of the replay
+service.
+
+## Triage outcome — constructor-arity deployment bug
+
+**Root cause: the backtest harness built the assertion create data with a one-argument
+`constructor(address)` payload, but the assertion's constructor is the two-argument
+`constructor(address, bool)`.**
+
+The one-argument payload — `creationCode || abi.encode(address(router))` — **reverts during
+construction**. The compiler-generated constructor prologue ABI-decodes its arguments from the
+code-appended tail and reverts when that tail is too short to contain the trailing `bool`. It
+does **not** silently read the missing `bool` as zero. Under `pcl`/`cl.assertion` this surfaces
+as `AssertionContractDeployFailed` before the triggered `router.swap` ever runs.
+
+The router at `0x6131B5fae19EA4f9D964eAc0408E4408b66337b5` genuinely is the original router
+family, so the correct deployment is `constructor(address, bool)` with
+`originalRouterFamily_ = true`. Deployed correctly, the assertion does not trip on this traffic.
+
+Answer to "should it have triggered in the first place?" — **No.** A correctly deployed
+assertion does not trip on this window. The trip was produced by the wrong constructor arity in
+the harness, which is a deployment failure rather than a fired assertion.
+
+## What this PR changes
+
+- Adds `testDeployment_OneArgPayload_RevertsDuringConstruction`: builds the exact one-argument
+  harness payload and asserts `CREATE` returns `address(0)` (construction reverted), while the
+  correct two-argument payload constructs. This pins the real root cause at the construction
+  layer, which is CI-safe (the `cl.assertion` deploy failure is an uncatchable process abort, not
+  a catchable Solidity revert).
+- Retains `testMinReturn_PartialFill_MisconfiguredFamilyFalse_Trips` as a *separate, explicit*
+  wrong-family behaviour test (a successfully-deployed assertion armed with
+  `originalRouterFamily_ = false`), clearly labelled as **not** the harness bug.
+- Keeps `testMinReturn_PartialFill_NoFalsePositive_Passes` as the correct-deployment A/B partner:
+  armed with `originalRouterFamily_ = true`, a legitimate partial-fill order credited below the
+  flat `minReturnAmount` passes because the `_PARTIAL_FILL` flag is skipped.
+- Gates the `kyber` profile in the examples CI matrix so this regression runs on every PR.
+
+## Reproducing the assertion IDs
+
+Assertion IDs are `keccak256(creationCode || encodedArgs)` and therefore depend on the compiled
+creation code and the exact toolchain. Do **not** hard-code them — regenerate from the bytes you
+actually submit:
+
+```sh
+# 1. Compile and read the creation code:
+FOUNDRY_PROFILE=kyber forge build
+#    .bytecode.object from
+#    examples/kyber/out/KyberMetaAggregationRouterAssertion.sol/KyberMetaAggregationRouterAssertion.json
+
+# 2. The correct (two-argument) constructor tail for the original router family:
+cast abi-encode "constructor(address,bool)" 0x6131B5fae19EA4f9D964eAc0408E4408b66337b5 true
+
+# 3. id = keccak256(creation_code || encoded_args)
+```
+
+The one-argument payload used by the buggy harness
+(`abi.encode(address(0x6131B5…))`, 32 bytes of tail) does not produce a working assertion — it
+fails construction, as pinned by `testDeployment_OneArgPayload_RevertsDuringConstruction`.
+
+## Reproducing the replay (optional)
+
+The source window was frozen at `start_block = 25480001`, `head = 25487144` (7143 blocks) and
+remained frozen at that range, so the run is reproducible against the Phylax assex-replay service
+(`chain_id = 1`). `POST /backtest` takes only `block_count` and scans forward from the fixed
+`start_block`; it stops on the first assertion match and, without a `callback_url`, exposes only
+`GET /backtest/{job}/progress` (an early `complete` = a match/trip). The service requires an
+archive node for the target chain; supply that RPC via the service configuration rather than
+embedding any credential here.
+
+A/B against the frozen window to confirm the deployment-arity root cause:
+
+- **Correct** (two-argument, `originalRouterFamily_ = true`): runs to `destination_block` with no
+  early stop — no trip.
+- **Buggy** (one-argument payload): the assertion fails to deploy, so the harness records a
+  deploy failure rather than a passing scan.
+
+The local `pcl test` A/B above already proves the mechanism deterministically without the live
+service.

examples/kyber/test/KyberMetaAggregationRouterAssertion.t.sol

@@ -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);
     }
 
@@ -375,6 +374,93 @@ 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 {
+        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;
+
+        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(
+            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
+    ///      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 {
+        // 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));
+        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);