Align AccountLens ABI and deployment sources - #85
Conversation
Reject unsuccessful or malformed ABI responses while keeping failed fetches retryable. Ignore whole-vault AccountLens query failures in account, rewards, and simulation results.
Resolving the AccountLens ABI through ABIService made every account read, reward-stream read, and simulation depend on fetching the ABI document: a 404, rate limit, or blocked host rejected before any RPC call, even though a usable bundled ABI ships with the package. Add resolveAccountLensAbi, which degrades to the bundled ABI when the document is unreachable or is missing the functions the SDK calls, and report the fallback where a channel exists (a FALLBACK_USED data issue on account reads, a warning log for rewards and simulation). Also: - keep the resolved ABI out of AccountLens query cache keys, so a ~33KB array is not serialized into every key (and into consumer query keys derived from the same helpers) - key the ABI request cache by resolved URL rather than contract name, so it tracks whatever getABIURL keys on - extract the duplicated SOURCE_UNAVAILABLE data issue in collectSettledVaultAccountInfos and document the two originalValue shapes - document why a whole-vault queryFailure is dropped silently in decodeAccountSnapshot: the EVC batch item succeeded, so it cannot surface via failedBatchItems Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- configure ABI and deployment reads from one interfaces branch - use the reward-specific AccountLens query and isolate failures - refresh the bundled fallback ABI for current lens deployments
📝 WalkthroughWalkthroughThe SDK adds configurable Euler Interfaces branch selection, dynamic deployment URLs, runtime AccountLens ABI loading with fallback and caching, ABI-aware account/reward/simulation reads, structured lens-read failure reporting, and legacy reward-query compatibility. ChangesAccountLens ABI integration
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
WalkthroughOverviewThis PR moves AccountLens account reads, layered transaction simulation, and reward-stream reads onto a runtime ABI sourced from the configured Incremental re-reviewReviewed head: Since the previous review:
Cross-repo composition
Rollout note: merge/publish this SDK and update consumers before or atomically with switching VerdictApproved. The two remaining code findings are resolved, and finding 1 is withdrawn under the accepted Non-blocking follow-upsCodeRabbit's branch-normalization note is valid but minor: programmatic config values containing whitespace are normalized by Validation
|
LeonardEulerXYZ
left a comment
LeonardEulerXYZ
left a comment
There was a problem hiding this comment.
Reviewed exact head ce02fbe5b085dccdeadfd6f9c36121ec7ea44741. Requesting changes for the default fallback-ABI incompatibility, silent partial simulation snapshots, and the deprecated setter's source-compatibility regression. Full validation: 492/492 tests passed; typecheck and build passed; lint exited 0 with existing warnings outside this diff. See inline comments for the concrete invariants and minimal fixes.
A failed AccountLens read dropped a position from a simulated snapshot with no way for callers to notice. `rawBatchResults` covers only the batch's action positions, so a reverted lens read never reaches `failedBatchItems`, and a whole-vault failure is reported in-band with the batch item itself succeeding. Either way `canExecute` could stay true while `simulatedAccounts` silently omitted a collateral or debt position, which a preflight consumer cannot distinguish from a complete healthy post-state. Record both failure modes and surface them on `SimulateBatchResult` as `snapshotReadFailures`, attributed per layer, sub-account and vault, with the lens `queryFailureReason` or the reverted read's return data. Positions still degrade to absent rather than throwing, matching the surrounding per-vault tolerance; `canExecute` still tracks whether the batch executes, and now documents the dependency on this field. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Renaming the reward reader to `queryRewardAccountInfo` removed a public property and changed the deprecated setter's callback contract, so existing TypeScript consumers failed to compile: direct access to `queryVaultAccountInfo` with TS2551, and an old-style `setQueryVaultAccountInfo` callback with TS2322 once `AccountRewardInfo` required `balanceTracker` and `balance`. Restore `queryVaultAccountInfo` with its original contract, deprecated and unused internally, and widen the deprecated setter to accept the legacy return shape, projecting it onto `AccountRewardInfo` so only the fields `fetchRewardStreams` actually reads are required. The setter keeps retargeting the live reader, as it always did — pointing it at the unused legacy property would silently discard an override. Note the rename itself was a fix: `getVaultAccountInfo` carries no `enabledRewardsInfo`, so the previous reader made `fetchRewardStreams` return an empty list unconditionally. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/euler-v2-sdk/test/simulate.test.ts (1)
1386-1391: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThis per-layer assertion is vacuous.
inBand.length > 0is already asserted at Line 1365, so the set size is always ≥ 1. Assert the actual expectation — every layer that read the vault reported it — e.g. compare againstresult.simulatedAccounts.length.💚 Suggested strengthening
- assert.ok( - new Set(inBand.map((failure) => failure.layerIndex)).size >= 1, - "expected per-layer attribution", - ); + assert.equal( + new Set(inBand.map((failure) => failure.layerIndex)).size, + result.simulatedAccounts.length, + "expected every layer to report the failure", + );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/euler-v2-sdk/test/simulate.test.ts` around lines 1386 - 1391, Strengthen the per-layer assertion in the simulate test by comparing the number of distinct failure.layerIndex values in inBand with result.simulatedAccounts.length. Replace the vacuous “size >= 1” check while preserving the existing per-layer attribution message.packages/euler-v2-sdk/src/services/accountService/adapters/accountOnchainAdapter/resolveAccountLensAbi.ts (1)
23-30: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low valueValidation only checks function names, not arity.
A runtime ABI whose
getVaultAccountInfotakes different inputs passes this check and then fails at encode/decode time with no fallback. Consider also asserting the expected input count for each required function so a genuinely incompatible document falls back to the bundle instead.♻️ Suggested tightening
-const DEFAULT_REQUIRED_FUNCTIONS = [ - "getEVCAccountInfo", - "getVaultAccountInfo", -] as const; +const DEFAULT_REQUIRED_FUNCTIONS = [ + "getEVCAccountInfo", + "getVaultAccountInfo", +] as const; +const EXPECTED_INPUT_COUNT = 2;requiredFunctions.filter( (name) => - !abi.some((item) => item.type === "function" && item.name === name), + !abi.some( + (item) => + item.type === "function" && + item.name === name && + item.inputs.length === EXPECTED_INPUT_COUNT, + ), );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/euler-v2-sdk/src/services/accountService/adapters/accountOnchainAdapter/resolveAccountLensAbi.ts` around lines 23 - 30, Update missingFunctions to validate each required function’s expected input arity, not only its name, so incompatible ABI entries are treated as missing and trigger the existing bundled-ABI fallback. Use the required-function definitions or related symbols in resolveAccountLensAbi to obtain the expected counts, while preserving the current name-based validation for all other functions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/euler-v2-sdk/src/sdk/defaultConfig.ts`:
- Around line 110-113: Update getEulerInterfacesDeploymentsUrl to normalize its
branch input before interpolation: trim whitespace and fall back to the
established master default when the result is empty, matching ABIService
behavior and keeping ABI/deployments branch selection aligned.
---
Nitpick comments:
In
`@packages/euler-v2-sdk/src/services/accountService/adapters/accountOnchainAdapter/resolveAccountLensAbi.ts`:
- Around line 23-30: Update missingFunctions to validate each required
function’s expected input arity, not only its name, so incompatible ABI entries
are treated as missing and trigger the existing bundled-ABI fallback. Use the
required-function definitions or related symbols in resolveAccountLensAbi to
obtain the expected counts, while preserving the current name-based validation
for all other functions.
In `@packages/euler-v2-sdk/test/simulate.test.ts`:
- Around line 1386-1391: Strengthen the per-layer assertion in the simulate test
by comparing the number of distinct failure.layerIndex values in inBand with
result.simulatedAccounts.length. Replace the vacuous “size >= 1” check while
preserving the existing per-layer attribution message.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: euler-xyz/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 388459df-11c0-4869-8809-8651e08e2935
📒 Files selected for processing (19)
packages/euler-v2-sdk/docs/config-through-env.mdpackages/euler-v2-sdk/src/sdk/buildSDK.tspackages/euler-v2-sdk/src/sdk/config.tspackages/euler-v2-sdk/src/sdk/defaultConfig.tspackages/euler-v2-sdk/src/services/abiService/abiService.tspackages/euler-v2-sdk/src/services/abiService/index.tspackages/euler-v2-sdk/src/services/accountService/adapters/accountOnchainAdapter/abis/accountLensAbi.tspackages/euler-v2-sdk/src/services/accountService/adapters/accountOnchainAdapter/accountLensTypes.tspackages/euler-v2-sdk/src/services/accountService/adapters/accountOnchainAdapter/accountOnchainAdapter.tspackages/euler-v2-sdk/src/services/accountService/adapters/accountOnchainAdapter/resolveAccountLensAbi.tspackages/euler-v2-sdk/src/services/executionService/executionService.tspackages/euler-v2-sdk/src/services/executionService/index.tspackages/euler-v2-sdk/src/services/executionService/simulate.tspackages/euler-v2-sdk/src/services/rewardsService/index.tspackages/euler-v2-sdk/src/services/rewardsService/rewardsService.tspackages/euler-v2-sdk/test/accountLensAbiService.test.tspackages/euler-v2-sdk/test/rewardsService.test.tspackages/euler-v2-sdk/test/sdkConfig.test.tspackages/euler-v2-sdk/test/simulate.test.ts
LeonardEulerXYZ
left a comment
LeonardEulerXYZ
left a comment
There was a problem hiding this comment.
Re-reviewed exact head f797bca26c37c00af6440e8e64f352b2057075f5. The incomplete-snapshot diagnostic and RewardsService source-compatibility findings are resolved. Per Darek's direction, I am withdrawing the prior bundled/default ABI finding as a code blocker here: euler-interfaces#225 carries the matching ABI and AccountLens redeployments, its verification checks pass, and the new mainnet Lens answered all three target reads in a live fixture. Validation: 33 files / 497 tests passed; typecheck and build passed; lint exited 0 with existing out-of-diff warnings; strict downstream compatibility fixture passed. Approved with a release-order note: publish this SDK and update consumers before or atomically with switching euler-interfaces/master; Euler Lite development still pins SDK 1.2.1 and must also handle snapshotReadFailures. CodeRabbit's branch-normalization and ABI-arity notes remain reasonable non-blocking hardening.
3 participants
Summary
Changes
Test plan
Summary by CodeRabbit