Skip to content

fix(kuru): refresh drifted Router deployment pins - #199

Closed
ShadowOfTime1 wants to merge 3 commits into
nishuzumi:mainfrom
ShadowOfTime1:fix/kuru-deployment-pins
Closed

ShadowOfTime1 wants to merge 3 commits into
nishuzumi:mainfrom
ShadowOfTime1:fix/kuru-deployment-pins

Conversation

@ShadowOfTime1

Copy link
Copy Markdown
Contributor

The scheduled ABI check went red because the Kuru Router proxy was upgraded on Monad mainnet, so the ERC-1967 implementation and orderBookImplementation() template pinned in abis.json no longer match on chain (the two mismatches in #194).

What I did:

  • Re-read both on mainnet: the Router (0xd651...) ERC-1967 slot now resolves to 0xf1635175914acF4Db170395D524323225e1F1a04, and orderBookImplementation() now returns 0x5e3446c600524Be453bbCEFD46a9E4C9bE8899a0. Both match the values in the issue.
  • Before trusting the new deployments, confirmed the Moss-required surface survived the upgrade via eth_getCode selector search: anyToAnySwap (0xffa5210a), verifiedMarket (0x5f71a07c) and orderBookImplementation (0xa0416499) are present in the new Router implementation, and placeAndExecuteMarketBuy (0x7c51d6cf), placeAndExecuteMarketSell (0x532c46db), the Trade topic and the FlipOrderUpdated topic are present in the new template.
  • Updated abis.json and the verification record in abi-explorer.test.ts with a dated re-verification note.

Verification: pnpm build, typecheck and lint pass; the offline suite is 85 passing. The one failing test is the pre-existing Kuru mainnet > simulates a native swap live check (ROUTE_QUOTE_UNAVAILABLE), which fails on unchanged main too and looks like a separate market-quote issue this PR does not touch. The online explorer ABI comparison (test:abi:online) needs MONADSCAN_API_KEY, so it runs in CI.

Refs #194

@pillowtalk-Qy pillowtalk-Qy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking this deployment drift investigation and for recording the on-chain re-verification. I independently checked the exact head 711be375 against main@e958f7f.

The following parts are consistent with the current chain state:

  • the Router proxy slot resolves to 0xf1635175914acF4Db170395D524323225e1F1a04;
  • orderBookImplementation() resolves to 0x5e3446c600524Be453bbCEFD46a9E4C9bE8899a0;
  • the required Router selectors and OrderBook selectors/topics are present in the deployed bytecode at a fixed read block;
  • lint, build, typecheck and the Kuru focused suite pass apart from the same native quote failure reproduced on unchanged main (ROUTE_QUOTE_UNAVAILABLE), so I am not treating that baseline failure as a blocker for this pin-only PR.

I found one merge-blocking provenance issue:

The new Router implementation is not publicly source-verified at the time of review. The MonadScan page for 0xf1635175914acF4Db170395D524323225e1F1a04 currently still shows Verify and Publish, rather than a verified contract source. The PR's online test is named and implemented as a comparison against an explorer-verified implementation and calls fetchAbi(manifest.router.implementation, key ?? ""). With the repository key absent locally, I could only run the non-keyed checks; the keyed suite fails at the expected missing-key guard. More importantly, a key alone cannot provide the claimed independent ABI if the explorer has no verified ABI for this implementation.

Could you please do one of the following before approval:

  1. provide the hosted keyed result showing that MonadScan exposes a verified ABI for this exact implementation and that compareDeployedAbi passes; or
  2. if the implementation remains unverified, change the provenance/test wording and verification path to ADR 0007's honest degraded verification for this Router as well: record the exact read block, deployed bytecode evidence, the required selector/event surface, and live adapter evidence, without calling it an explorer-verified ABI cross-check or requiring fetchAbi() to succeed for this address.

The current pin update should remain limited to the deployment/provenance work. I am not asking to change the Kuru route comparison or to disable exhaustive behavior. Once the ABI evidence path is corrected and the exact head is rechecked, I can review the updated commit again.

@ShadowOfTime1

Copy link
Copy Markdown
Contributor Author

Thanks for the thorough review. You're right that option 1 isn't available — MonadScan has no verified source for this Router implementation — so I took option 2 and moved the Router to ADR 0007's honest degraded verification, the same shape the OrderBook template already uses.

The online suite no longer calls fetchAbi, no longer claims an explorer-verified cross-check, and no longer needs MONADSCAN_API_KEY. Instead it reads the implementation's deployed bytecode and asserts the Moss-required Router surface (the functions the adapter actually calls: anyToAnySwap, verifiedMarket, orderBookImplementation) is present, deriving each selector from the vendored ABI rather than trusting a hardcoded value. The header record now states plainly that neither the Router implementation nor the OrderBook template is source-verified, records the read block (mainnet 104120573) and the required selectors, and notes that bytecode presence is not source verification. I also dropped the now-unused allowedExplorerOnly manifest field.

The pin update is otherwise unchanged and I did not touch the Kuru route comparison. Verified at head f2581c4: lint, build, typecheck, the offline Kuru suite (84 passed, 2 skipped), and the online provenance suite (4 passed) against Monad mainnet.

@ShadowOfTime1

Copy link
Copy Markdown
Contributor Author

Heads-up on the red check here: it's the repo-wide CI condition tracked in #195 (the expired aave quarantine tripwire plus the first-fail bail), not anything in this PR. This change only touches the kuru package — lint, build, typecheck and the offline suite pass, and the online Kuru provenance suite passes against mainnet. Once #195 isolates the per-package signal this PR should read green on its own. Happy to rebase whenever that lands.

@ShadowOfTime1

Copy link
Copy Markdown
Contributor Author

@pillowtalk-Qy following up — I addressed your review a week ago (Sep 12): the Router provenance now uses ADR 0007 honest degraded verification (dropped the explorer-verified claim and the MONADSCAN key; it checks the required Router selectors, derived from the vendored ABI, against the deployed bytecode) and I replied to each point on the thread. Could you take another look and clear the review if it looks good? The change is scoped to the kuru package and its own lint/build/typecheck/offline checks pass. Thanks!

@nishuzumi nishuzumi left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked exact head f2581c43 against main@4e3b985. The pin values are right — the Router slot resolves to 0xf163…1a04 and orderBookImplementation() to 0x5e34…99a0 today as well — and moving the Router to ADR 0007's degraded verification is the correct response to an unverified implementation. Three things before this can merge; the first two are small.

  1. Derive the selectors from the vendored ABI, not from hand-typed strings. REQUIRED_ROUTER_FUNCTIONS (test-online/abi-explorer.test.ts:89-93) carries three handwritten signatures; the test checks only that a function named anyToAnySwap exists in KuruRouterAbi and then hashes the handwritten string. If the vendored ABI's parameter list ever differed from the string, the test would still pass against bytecode that matches the string and not the artifact the adapter actually encodes with. Find the entry in KuruRouterAbi and call toFunctionSelector(entry) on it; drop the string list (or keep it only as an expect(toFunctionSelector(entry)).toBe(toFunctionSelector(signature)) cross-check).

  2. Turn the OrderBook record into a test. The Receipt parsers decode Trade, FlipOrderUpdated and FlippedOrderCreated, and the Capability calls placeAndExecuteMarketBuy/Sell. Those five selectors/topics are listed in the header comment as "confirmed present" in template 0x5e34…, but nothing asserts it — and the only source-verified market implementation this package ever compared against is now historical. Same shape as the Router test: getCode(manifest.orderBook.expectedTemplateImplementation) and assert the two function selectors and three event topics, derived from KuruOrderBookAbi. Record the read block in abis.json (e.g. "verifiedAtBlock") so the record is reproducible rather than narrated.

  3. The third leg of degraded verification is red. ADR 0007 line 18 requires that the adapter's live behaviour be exercised on mainnet; the Kuru native-swap live test has failed since 2026-09-08 with ROUTE_QUOTE_UNAVAILABLE, and I reproduced the underlying revert again today: placeAndExecuteMarketSell on the verified MON/USDC market 0x764b…29D5 (running the new template) returns 0x004b65ba = MarketStateError(). Two weeks is not transient book liquidity. Until we know why that market rejects quotes under the new template, the new pins are verified for surface, not for behaviour. This PR does not have to fix that itself, but it should not claim live evidence it does not have: please replace the "exercise the adapter's live behavior" clause in the header with the current fact (the MON/USDC live smoke fails with MarketStateError; tracked in #194/#205), so the record is honest.

patch changeset is right for a pin refresh. I will re-review the new head promptly; the deployment drift has kept the ABI cross-check workflow red since 2026-09-02, so I want this in.

The Router proxy was upgraded on Monad mainnet, so abis.json's recorded
ERC-1967 implementation and orderBookImplementation() template no longer
matched on chain and the scheduled ABI check failed. Re-read both from the
ERC-1967 slot and orderBookImplementation() on mainnet, confirmed the
Moss-required surface is present in the new bytecode (anyToAnySwap,
verifiedMarket, orderBookImplementation on the Router; placeAndExecuteMarketBuy,
placeAndExecuteMarketSell and the Trade/FlipOrderUpdated topics on the
template), then updated the pins and the verification record.

Refs nishuzumi#194
…ification

The refreshed Router implementation is not source-verified on MonadScan, so the
explorer-ABI cross-check (fetchAbi + compareDeployedAbi against an
explorer-verified implementation) cannot hold: there is no verified ABI to
fetch for this address. Per ADR 0007, degrade honestly instead. The online
suite now reads the implementation's deployed bytecode and asserts the
Moss-required Router selectors, derived from the vendored ABI, are present; it
no longer claims an explorer-verified cross-check or needs MONADSCAN_API_KEY.
The record notes the read block (mainnet 104120573) and the required selectors.
Drop the now-unused allowedExplorerOnly manifest field.

Verified: lint, build, typecheck, the offline Kuru suite (84 passed, 2 skipped),
and the online provenance suite (4 passed) against Monad mainnet.
…rderBook surface

Address review on nishuzumi#199:
- Derive every Router selector from the vendored ABI entry (toFunctionSelector
  on the artifact) and keep the human-readable signature only as a cross-check,
  so a parameter-list drift between the ABI and the bytecode cannot pass.
- Add the OrderBook counterpart the header only narrated: read the market
  template bytecode and assert the two dispatcher selectors and three event
  topics, each derived from KuruOrderbookAbi. Record the read block as
  verifiedAtBlock in abis.json and pin the bytecode reads to it.
- Drop the header's claim to exercise live behaviour: that leg is red (the
  MON/USDC native-swap smoke fails with MarketStateError on one of four verified
  markets; tracked in nishuzumi#194/nishuzumi#205), so the record states the fact instead.
@ShadowOfTime1
ShadowOfTime1 force-pushed the fix/kuru-deployment-pins branch from f2581c4 to f9da64e Compare September 22, 2026 07:47
@ShadowOfTime1

Copy link
Copy Markdown
Contributor Author

Thanks — all three addressed at f9da64e (rebased onto current main).

  1. Selectors derived from the ABI. Each Router selector now comes from toFunctionSelector(abiFunction(KuruRouterAbi, name)) — the vendored entry, not a string. The human-readable signature stays only as the expect(...).toBe(toFunctionSelector(signature)) cross-check you suggested, so a parameter-list drift between the artifact and the bytecode fails the test. (getAbiItem with a runtime name blows up TS with a deep-instantiation error, so the lookup is a small abiFunction/abiEvent helper over viem's Abi — no cast.)

  2. OrderBook record is now a test. New case reads getCode(orderBook.expectedTemplateImplementation) and asserts the two dispatcher selectors and the three event topics, each derived from KuruOrderbookAbi. verifiedAtBlock is recorded in abis.json and both bytecode reads pin to it, so the record reproduces rather than narrates.

  3. Header no longer claims live behaviour. Dropped the "exercise the adapter's live behavior" clause; the header now states the fact — the MON/USDC native-swap smoke fails with MarketStateError() on one of the four verified markets, tracked in Kuru: deployment pins have drifted; native swap live test fails during route comparison #194/fix(kuru): read MarketStateError as a market that is not trading #205 — so this PR verifies surface, not behaviour.

While here I checked the behaviour question for #205: of the four Router-verified MON/USDC markets, three fill and only 0x764b…29D5 reverts MarketStateError(), all four on the new template — so the template is fine and that one market is individually stuck. Details on #205.

Verify at this head: pnpm lint, pnpm build, pnpm typecheck, offline kuru (84/2 skipped), and test:abi:online 5/5 against mainnet.

@ShadowOfTime1

Copy link
Copy Markdown
Contributor Author

Closing this to free my claim slot for other pool work. The branch stays on my fork and the change is ready on my side, so it can be reopened as is if it's still wanted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants