perf: eliminate redundant read_dispair calls in try_into_call - #2793
perf: eliminate redundant read_dispair calls in try_into_call#2793thedavidmeister wants to merge 9 commits into
Conversation
try_into_call previously called read_dispair 3 times (once directly and once inside each try_parse_rainlang call), totalling 12 sequential RPC round-trips just to read 4 immutable deployer addresses. Add parse_bytecode_with_dispair, which takes a pre-fetched DISPaiR and skips the read_dispair step. try_parse_rainlang becomes a wrapper around read_dispair + parse_bytecode_with_dispair. try_into_call now calls read_dispair once and passes the result to both parse calls via clone/move. RPC calls for a single deposit+addOrder submission: 14 → 6 (saves 8 sequential round-trips, ~2.4s at 300ms RTT). Adds assertions to test_into_add_order_call verifying both config.evaluable and tasks[0].evaluable carry the correct interpreter and store addresses from the single shared dispair read. Closes #2776 Co-Authored-By: Claude <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughOrder preparation now reuses a single DISPaiR fetch, carries token decimals through deposit processing, and reuses complete recorded token metadata before on-chain lookup. Tests validate shared evaluable addresses, approval scaling, and token-info fetch reuse. ChangesOrder preparation RPC reuse
Estimated code review effort: 3 (Moderate) | ~25 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. 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 |
Mark try_parse_rainlang as #[cfg(test)] — after the try_into_call refactor it is only called from tests, so the production build flags it as dead code under -D warnings. Co-Authored-By: Claude <noreply@anthropic.com>
|
🤖 ai:vetter |
|
Rework note (human): incomplete coverage of #2776. The issue names three redundant RPC-fetch sources; this PR dedups only |
…decimals fetches set_select_token reuses a complete token record already in the yaml for the same network+address (recorded by an earlier selection) instead of refetching token_info, so selecting one token for both the input and output slot costs a single multicall (2 -> 1). Incomplete records and records on other networks never shortcut the fetch, and re-selecting a key with its current address also skips it. generate_approval_calldatas reads decimals from the deposits map (which now carries them from get_token_info) instead of issuing a decimals() RPC per token (1 -> 0); the only remaining round-trip per token is the allowance read. This also aligns the approval amount's decimals source with the deposit calldata path (yaml-first with on-chain fallback). Both behaviors are pinned by mock hit-count tests, mutation-validated against reintroduced duplicate fetches, dropped network/address/ completeness guards, lookup-after-removal ordering, and corrupted map decimals. Co-Authored-By: Claude <noreply@anthropic.com>
|
🤖 ai:producer |
…lldata dedup [3b-attempt] generate_approval_calldatas no longer issues a per-token decimals() RPC (decimals come from the deposits map), so the sequence-based mocked decimals responses in the approval tests were consumed by the allowance reads instead, turning a 1000e18 allowance into 18 wei and producing an extra approval calldata (AssertionError: 2 == 1 at builder.test.ts:1463). Remove the now-dead decimals mocks so each deposited token mocks exactly its allowance read. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
🤖 ai:producer |
|
🤖 ai:producer |
|
🤖 ai:vetter |
Decimals for approval calldatas now come from the deposits map (yaml records: token1=6, token2=18), not a mocked 18-decimals RPC. token1's allowance mock must therefore be 1000e6 to equal its 1000 deposit and be skipped, leaving the single expected token2 approval. Each allowance mock is also pinned to its token address so the unspecified deposit-map iteration order cannot cross-wire the responses. Co-Authored-By: Claude <noreply@anthropic.com>
|
🤖 ai:vetter |
|
🤖 ai:vetter |
Summary
try_into_callpreviously calledread_dispairthree times — once inline and once inside eachtry_parse_rainlangcall — totalling 12 sequential RPC round-trips just to read 4 immutable deployer addresses. At 300ms RTT (remote RPC) that is ~3.6s of avoidable latency.Changes
parse_bytecode_with_dispair: the parsing body extracted fromtry_parse_rainlang, takes a pre-fetchedDISPaiR(no additional dispair reads).try_parse_rainlangis now a thin wrapper:read_dispair+parse_bytecode_with_dispair. All existing tests remain unchanged.try_into_callcallsread_dispaironce, clones for the first parse, moves for the second parse, and extractsinterpreter/storeupfront.Result
RPC calls per
generate_deposit_and_add_order_calldatas(dispair portion):read_dispair)read_dispair)Total for a single deposit+addOrder: 14 → 6 sequential
eth_callround-trips, saving ~2.4s at 300ms RTT.Test
Adds assertions to
test_into_add_order_callverifying bothconfig.evaluable.interpreter/storeandtasks[0].evaluable.interpreter/storematch the expected interpreter/store addresses (and match each other). This mutation-validates that both evaluables receive the correct dispair from the single shared read.Closes #2776
Co-Authored-By: Claude noreply@anthropic.com
Summary by CodeRabbit