fix: make v6 subgraph queries and dispair resolution work against RaindexV6 - #481
Open
hardyjosh wants to merge 1 commit into
Open
fix: make v6 subgraph queries and dispair resolution work against RaindexV6#481hardyjosh wants to merge 1 commit into
hardyjosh wants to merge 1 commit into
Conversation
…ndexV6 Three things stopped the solver from trading a v6 orderbook on a chain whose only DEX is UniswapV3. All were hit end to end on Robinhood Chain (4663). 1. The v6 subgraph queries ask for an entity that no longer exists. config.example.yaml claims the v5 and v6 subgraph schemas are identical and that the `v6=` prefix exists only to pick the order decoder. That is no longer true: the v6 schema renamed the orderbook entity to `raindex`, along with `orderbooks` -> `raindices` and the `orderbook_in` / `orderbook_not_in` filter keys -> `raindex_in` / `raindex_not_in`. Every v6 query therefore failed with "Type `Order` has no field `orderbook`", surfaced as the much less obvious "Failed to fetch orders / Received invalid response". The query builders now take the subgraph version and select the v6 entity under an `orderbook` alias, so the response shape is unchanged and no consumer of SgOrder / SgTransaction had to move. 2. Dispair resolution assumes getters the RaindexV6 deployer does not have. resolveVersionContracts derives the interpreter and store by calling I_INTERPRETER() / I_STORE() on the configured deployer. The RaindexV6-era deployer exposes only parse2, parsePragma1, describedByMetaV1, buildIntegrityFunctionPointers and supportsInterface, so both reads revert, contracts.v6 comes back undefined, and every order is skipped with "Cannot trade as dispair addresses are not configured for order V4 trade" even though dispair is configured. `contracts.<version>.interpreter` and `.store` are now optional overrides. When given they are used directly; otherwise the onchain reads happen exactly as before, so existing configs are unaffected. 3. BASES_TO_CHECK_TRADES_AGAINST was indexed unguarded. getCounterpartyOrdersAgainstBaseTokens throws a TypeError the first time it enumerates intra-orderbook counterparties on any chain sushi has no routing bases for. Guarded with `?? []`. Also bumps the sushi submodule to the Robinhood Chain (4663) support branch. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HiqQdxokJ4edjAFyAkN9G3
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (13)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. WalkthroughThe change adds optional interpreter and store contract overrides, resolves missing addresses from chain data, adds v6-aware subgraph queries, handles missing base-token configuration, and updates the Sushiswap submodule. ChangesContract address overrides
Version-aware subgraph queries
Base-token filtering
Sushiswap submodule update
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Bug fix · Severity of issue fixed: Low Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant SubgraphIndex
participant QueryBuilder
participant SubgraphAPI
SubgraphIndex->>QueryBuilder: provide detected subgraph version
QueryBuilder->>QueryBuilder: build legacy or v6 GraphQL query
SubgraphIndex->>SubgraphAPI: submit generated query
Merge Risk: ⚪ Minimal · up to No actionable merge-blocking risk remains in the reviewed changes. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 11 files. (2 skipped: 2 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three things stopped the solver from trading a v6 orderbook on a chain whose only DEX is UniswapV3. All were hit end to end on Robinhood Chain (4663), and with this branch the solver now finds a UniswapV3 route there and fills a live Raindex v6 order.
Depends on rainlanguage/sushiswap#51 (chain 4663 support); the submodule bump points at that branch.
1. The v6 subgraph queries ask for an entity that no longer exists
config.example.yamlsays the v5 and v6 subgraph schemas are identical and that thev6=prefix exists only to pick the order decoder. That is no longer true. The v6 schema renamed the orderbook entity:Order.orderbookOrder.raindexorderbooks(root)raindicesorderbook_in/orderbook_not_inraindex_in/raindex_not_inDeposit.orderbook,Withdrawal.orderbook,TradeVaultBalanceChange.orderbook.raindexSo every v6 query fails with
Type `Order` has no field `orderbook`, which the solver surfaces as the much less obviousFailed to fetch orders / Received invalid response.getOrderbooks()silently returned an empty set for the same reason.The three query builders now take a
SubgraphVersionsand select the v6 entity under anorderbookalias, so the response shape is byte-identical across versions and no consumer ofSgOrder/SgTransactionhad to move. Legacy behaviour is the default parameter value.2. Dispair resolution assumes getters the RaindexV6 deployer does not have
resolveVersionContractsderives the interpreter and store by callingI_INTERPRETER()/I_STORE()on the configured deployer. The RaindexV6-era deployer (0x7219E7497e31DD99411C06Cab9AD9a155E23369con 4663) exposes only five selectors:Both reads revert,
contracts.v6comes backundefined, and every order is skipped withCannot trade as dispair addresses are not configured for order V4 trade— even thoughdispairis configured, which makes the message actively misleading.contracts.<version>.interpreterand.storeare now optional config overrides. When supplied they are used directly; otherwise the onchain reads happen exactly as before, so existing configs are untouched.3.
BASES_TO_CHECK_TRADES_AGAINSTwas indexed unguardedgetCounterpartyOrdersAgainstBaseTokensdoesBASES_TO_CHECK_TRADES_AGAINST[chainId].every(...). That map is typed{ readonly [chainId: number]: Token[] }andgetChainConfigdoes not validate it, so on any chain sushi has no routing bases for this throws aTypeErrorthe first time intra-orderbook counterparties are enumerated. Guarded with?? [].Testing
npm run lintclean,npm run unit-test1094 passed / 66 files.resolveVersionContractspreferring configured interpreter/store (asserting the deployer is not read) and falling back per-field;getCounterpartyOrdersAgainstBaseTokenson a chain with no bases entry.?? []guard fails the new test withTypeError: Cannot read properties of undefined (reading 'every').RouteProcessorRaindexV6ArbOrderTaker, emptying the 20 USDG vault.🤖 Generated with Claude Code
https://claude.ai/code/session_01HiqQdxokJ4edjAFyAkN9G3
Summary by CodeRabbit
New Features
Bug Fixes
Tests
CI note
Deploy-Previewis red, but it is red on every one of the last 10 runs across every branch (gh run list --workflow deploy-preview.yml), includingmaster. It dies 4s in withAll subgraphs have indexing errorfrom the preview env's own config, not from anything here. Every other check is green, including all 7 e2e fork chains andgit-clean.(
e2e fork test (BASE)failed once on the first run and passed on rerun. Root cause is a pre-existing 1-in-256 flake in the harness, not this diff:test/utils.jsrandomUint256()returns a zero-padded 32-byte hex, bute2e.test.jswraps it inethers.BigNumber.from(...), which strips a leading zero byte — so whenever the first random byte is0x00the v5 orderbook'sbytes32 vaultIdcoder rejects it withincorrect data length. Worth fixing separately by keeping the vaultId as a padded hex string rather than a BigNumber.)