fix(sdk): resolve AccountLens ABI through ABIService - #82
fix(sdk): resolve AccountLens ABI through ABIService#82LeonardEulerXYZ wants to merge 3 commits into
Conversation
📝 WalkthroughWalkthroughThe SDK now resolves AccountLens ABIs at runtime, validates and caches ABI requests, falls back to bundled ABIs when needed, and propagates the resolved ABI through account reads, simulations, rewards, and SDK service wiring. ChangesAccountLens ABI integration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant SDK
participant ABIService
participant AccountOnchainAdapter
participant ExecutionService
participant RewardsService
participant Provider
SDK->>AccountOnchainAdapter: inject ABIService
SDK->>ExecutionService: setABIService
SDK->>RewardsService: setABIService
AccountOnchainAdapter->>ABIService: resolve AccountLens ABI
ABIService-->>AccountOnchainAdapter: runtime or bundled ABI
AccountOnchainAdapter->>Provider: readContract with resolved ABI
ExecutionService->>ABIService: resolve AccountLens ABI
ABIService-->>ExecutionService: runtime or bundled ABI
ExecutionService->>Provider: simulate lens batch and decode snapshot
RewardsService->>ABIService: resolve AccountLens ABI
ABIService-->>RewardsService: runtime or bundled ABI
RewardsService->>Provider: readContract with resolved ABI
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
Reject unsuccessful or malformed ABI responses while keeping failed fetches retryable. Ignore whole-vault AccountLens query failures in account, rewards, and simulation results.
LeonardEulerXYZ
left a comment
LeonardEulerXYZ
left a comment
There was a problem hiding this comment.
Code review
Verdict: blocking address/ABI rollout issue remains.
The implementation consistently threads one resolved ABI through account reads, reward-stream reads, and simulation encoding/decoding. The cache coalescing and failed-request eviction are also sound in the directly tested path.
The unresolved problem is composition with euler-interfaces#225: the deployment address and ABI are still obtained through separate mutable master reads, while the bundled fallback remains the legacy ABI. This leaves both new and already-published clients exposed to incompatible pairings.
A live mainnet fixture against vault 0x0120c2748545a4D9C875CDdFB439f786d6f1B460 confirmed the full mismatch matrix:
- legacy address + legacy ABI: correct decode;
- replacement address + replacement ABI: correct decode;
- legacy address + replacement ABI: decode throws;
- replacement address + legacy ABI: silently decodes corrupt fields (
vaultbecame0x000000000000000000000000000000006A68c197andassetsAccountbecame an absurd shifted value).
This can occur for a new SDK instance constructed before the interfaces update but first used afterward, and it is guaranteed for old SDK bundles once PR #225 replaces the mutable accountLens address. The PR body correctly mentions the boundary, but the linked rollout does not yet implement the required preservation/versioning.
Before rollout, bind the address and ABI atomically/versionedly (for example, preserve the legacy accountLens field and add a new versioned field consumed only by compatible SDKs, or use an immutable manifest/commit containing both). The optional bundled fallback should only be used with a deployment known to match it; otherwise require an explicitly matching ABI service and fail closed. Add a regression that decodes real old/new tuple payloads and exercises both mismatched pairings—the current tests assert ABI plumbing by object identity but do not discriminate this failure.
Validation on head 529effe7:
- focused ABI/simulation tests: 27 passed;
- full SDK suite: 482 passed;
- typecheck: passed;
- build: passed;
- lint: passed with five warnings in unchanged files;
git diff --check: passed;- live mainnet old/new AccountLens composition probe: reproduced the incompatibility above;
- whole-diff executable/supply-chain sweep: no unrelated suspicious changes found.
This PR is authored by the authenticated LeonardEulerXYZ account, so this is a comment review rather than an approval/request-changes event.
| const evc = deployment.addresses.coreAddrs.evc; | ||
| const resolvedAccountLensAbi = | ||
| (await this.abiService?.fetchABI(chainId, "AccountLens")) ?? | ||
| accountLensAbi; |
There was a problem hiding this comment.
Blocking rollout issue: this fallback is not safe once euler-interfaces#225 replaces lensAddrs.accountLens. A live call to the deployed replacement mainnet lens decoded with the bundled legacy ABI without throwing, but shifted every field (vault became 0x000…6A68c197, while assetsAccount became a huge value). The inverse pairing—legacy address with the new ABI—throws. Because the address and ABI are fetched independently from mutable master, even a newly published SDK can observe either mismatch; already-published SDKs will necessarily use this legacy ABI with the new address. Please version/bind the address+ABI pair and only allow this fallback for a deployment known to match it, rather than treating the fallback as generally backward-compatible.
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>
|
Superseded by #85, which carries the same commit history on the first-party fix/account-lens-runtime-sources branch and includes the latest runtime ABI, deployment-source, and reward-query fixes. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/euler-v2-sdk/src/services/executionService/simulate.ts (1)
467-473: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFallback warning fires on every simulation.
simulateTransactionPlanruns per quote/refresh, and a failed ABI fetch stays failed for the failure-cache window, so this can emit a warning on every call. Consider deduping (warn once per chain+reason) to keep the console usable.🤖 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/executionService/simulate.ts` around lines 467 - 473, Deduplicate the fallback warning in simulateTransactionPlan so the same chainId and fallbackReason are logged only once during the failure-cache window. Update the fallbackReason handling around resolveAccountLensAbi, using persistent module- or service-scoped state, while preserving the existing fallback behavior and warning content.packages/euler-v2-sdk/src/services/accountService/adapters/accountOnchainAdapter/resolveAccountLensAbi.ts (1)
23-27: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win
missingFunctionsonly checks names, not signatures.A runtime ABI whose
getVaultAccountInfoinputs/outputs changed still passes validation, so the mismatch surfaces only as a decode failure (or a silently shifted result) at read time. Consider also asserting the expected input arity/types for the two functions so a reshaped lens is detected here, where the fallback reason can be reported.♻️ Sketch: validate inputs too
-const REQUIRED_FUNCTIONS = [ - "getEVCAccountInfo", - "getVaultAccountInfo", -] as const; +const REQUIRED_FUNCTIONS = [ + { name: "getEVCAccountInfo", inputs: ["address", "address"] }, + { name: "getVaultAccountInfo", inputs: ["address", "address"] }, +] as const; const missingFunctions = (abi: Abi): string[] => REQUIRED_FUNCTIONS.filter( - (name) => - !abi.some((item) => item.type === "function" && item.name === name), - ); + (required) => + !abi.some( + (item) => + item.type === "function" && + item.name === required.name && + item.inputs.length === required.inputs.length && + item.inputs.every((input, i) => input.type === required.inputs[i]), + ), + ).map((required) => required.name);🤖 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 - 27, Update missingFunctions to validate the expected signatures of the required lens functions, not just their names: assert the input arity and types for getVaultAccountInfo and the other required function, while preserving the existing missing-name behavior. Treat any signature mismatch as missing so the adapter falls back before read-time decoding.
🤖 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/services/accountService/adapters/accountOnchainAdapter/accountOnchainAdapter.ts`:
- Around line 122-133: Update getQueryKeyEVCAccountInfo and the corresponding
key construction around the additionally affected range to include a small
discriminator indicating whether the bundled fallback ABI was used. Keep the ABI
array itself out of the key, but ensure fallback-decoded and runtime-ABI-decoded
results produce distinct cache keys while preserving the existing provider,
address, and sub-account components.
In `@packages/euler-v2-sdk/src/services/executionService/simulate.ts`:
- Around line 863-873: Log in-band query failures before dropping the result. In
packages/euler-v2-sdk/src/services/executionService/simulate.ts:863-873, update
the vaultAccount queryFailure branch to warn with meta.subAccount, meta.vault,
and queryFailureReason before continue; in
packages/euler-v2-sdk/src/services/rewardsService/rewardsService.ts:616-627,
update the queryFailure branch to warn with the vault and queryFailureReason
before returning [].
---
Nitpick comments:
In
`@packages/euler-v2-sdk/src/services/accountService/adapters/accountOnchainAdapter/resolveAccountLensAbi.ts`:
- Around line 23-27: Update missingFunctions to validate the expected signatures
of the required lens functions, not just their names: assert the input arity and
types for getVaultAccountInfo and the other required function, while preserving
the existing missing-name behavior. Treat any signature mismatch as missing so
the adapter falls back before read-time decoding.
In `@packages/euler-v2-sdk/src/services/executionService/simulate.ts`:
- Around line 467-473: Deduplicate the fallback warning in
simulateTransactionPlan so the same chainId and fallbackReason are logged only
once during the failure-cache window. Update the fallbackReason handling around
resolveAccountLensAbi, using persistent module- or service-scoped state, while
preserving the existing fallback behavior and warning content.
🪄 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: 3baf5823-60d3-4066-9e12-b2df8ea417c2
📒 Files selected for processing (10)
packages/euler-v2-sdk/src/sdk/buildSDK.tspackages/euler-v2-sdk/src/services/abiService/abiService.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/simulate.tspackages/euler-v2-sdk/src/services/rewardsService/rewardsService.tspackages/euler-v2-sdk/test/accountLensAbiService.test.tspackages/euler-v2-sdk/test/simulate.test.ts
| // The resolved ABI is deliberately not part of the key: it is fetched once and | ||
| // memoized for the lifetime of the ABIService, so it is constant for every read | ||
| // keyed here, and serializing a ~33KB array into every key is pure overhead. | ||
| getQueryKeyEVCAccountInfo( | ||
| provider: ReturnType<ProviderService["getProvider"]>, | ||
| accountLensAddress: Address, | ||
| evc: Address, | ||
| subAccount: Address, | ||
| _abi?: Abi, | ||
| ): string | null { | ||
| return serializeQueryArgs([provider, accountLensAddress, evc, subAccount]); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Cache key assumes the resolved ABI never changes within one process.
The comment holds only while resolution outcome is stable. ABIService.fetchABI now evicts failed requests so a later call retries, so an early fetchSubAccount can read with the bundled fallback while a later one reads with the runtime ABI — both keyed identically, so the fallback-decoded result is served from cache for the remaining TTL. A cheap fix is to fold a small discriminator (e.g. whether the fallback was used) into the key rather than the whole array.
🔧 Suggested key discriminator
getQueryKeyVaultAccountInfo(
provider: ReturnType<ProviderService["getProvider"]>,
accountLensAddress: Address,
subAccount: Address,
vault: Address,
- _abi?: Abi,
+ abi?: Abi,
): string | null {
return serializeQueryArgs([
provider,
accountLensAddress,
subAccount,
vault,
+ abi === bundledAccountLensAbi ? "bundled" : "runtime",
]);
}Also applies to: 154-168
🤖 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/accountOnchainAdapter.ts`
around lines 122 - 133, Update getQueryKeyEVCAccountInfo and the corresponding
key construction around the additionally affected range to include a small
discriminator indicating whether the bundled fallback ABI was used. Keep the ABI
array itself out of the key, but ensure fallback-decoded and runtime-ABI-decoded
results produce distinct cache keys while preserving the existing provider,
address, and sub-account components.
| if (meta.kind === "vaultAccount") { | ||
| const decodedVaultInfo = decodeFunctionResult({ | ||
| abi: accountLensAbi, | ||
| abi: resolvedAccountLensAbi, | ||
| functionName: "getVaultAccountInfo", | ||
| data: resultItem.result, | ||
| }) as unknown as VaultAccountInfo; | ||
| // The lens reports a whole-vault query failure in-band, so the EVC batch | ||
| // item itself succeeded and this will not appear in `failedBatchItems`. | ||
| // Drop the position, matching how a failed batch item is skipped above: | ||
| // a partial snapshot degrades to "position absent" rather than throwing. | ||
| if (decodedVaultInfo.queryFailure) continue; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
In-band AccountLens queryFailure is swallowed on both result shapes that lack a diagnostics channel. The EVC batch item / read succeeds, so nothing reaches failedBatchItems or a DataIssue; the data just disappears and is indistinguishable from an empty position. The account adapter path reports these as SOURCE_UNAVAILABLE — these two paths should at least log the reason, mirroring the ABI-fallback warnings already added in both functions.
packages/euler-v2-sdk/src/services/executionService/simulate.ts#L863-L873: beforecontinue, warn withmeta.subAccount,meta.vault, andqueryFailureReasonso a dropped position that skewscanExecute/snapshots is visible.packages/euler-v2-sdk/src/services/rewardsService/rewardsService.ts#L616-L627: in thequeryFailurebranch, warn with the vault andqueryFailureReasonbefore returning[].
📍 Affects 2 files
packages/euler-v2-sdk/src/services/executionService/simulate.ts#L863-L873(this comment)packages/euler-v2-sdk/src/services/rewardsService/rewardsService.ts#L616-L627
🤖 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/executionService/simulate.ts` around lines
863 - 873, Log in-band query failures before dropping the result. In
packages/euler-v2-sdk/src/services/executionService/simulate.ts:863-873, update
the vaultAccount queryFailure branch to warn with meta.subAccount, meta.vault,
and queryFailureReason before continue; in
packages/euler-v2-sdk/src/services/rewardsService/rewardsService.ts:616-627,
update the queryFailure branch to warn with the vault and queryFailureReason
before returning [].
2 participants
Summary
AccountLensABI through the existingABIServicefor onchain account reads, reward-stream reads, and execution simulationABIServiceWhy
The SDK currently combines two different update models:
euler-interfacesdeployment documentAccountLensABI is compiled into each published SDK packageIf
euler-interfacesreplaces the deployed AccountLens address with a contract whose return tuple changed, an existing SDK can call the new address and decode it with the old bundled ABI. In particular, adding leading fields toVaultAccountInfokeeps the function selector unchanged but changes the return-data layout.This change makes newly published SDK versions resolve the runtime ABI from the same interface repository used for deployment metadata, instead of leaving AccountLens reads tied exclusively to the bundled copy.
Type compatibility
ABIServicereturns viem's broad runtimeAbitype, so viem cannot preserve literal-ABI inference at these call sites. The runtime ABI is therefore contained inside the existing adapters and simulation implementation, while the SDK's stable domain result types (EVCAccountInfo,VaultAccountInfo, account entities, and reward-stream entities) remain unchanged.Existing helper and constructor call sites remain source-compatible:
AccountOnchainAdapterreceivesABIServiceas a new optional trailing constructor argumentRollout boundary
This PR protects consumers of a newly published SDK version. It cannot repair already-published SDK bundles that continue to fetch mutable deployment addresses while carrying an older ABI.
The corresponding
euler-interfacesrollout must therefore preserve/version the legacy AccountLens deployment for old clients rather than silently replacing an ABI-incompatible address. Also, the ABI and deployment documents are still separatemasterreads; a follow-up versioned manifest or immutable commit reference is required for strict atomic address/ABI coupling.Consumers using a custom
deploymentsUrl, deployment-service override, or explicit reward-streamaccountLensAddressmust also provide a matching customabiService; otherwise the custom address will still be paired with the ABI fetched fromeuler-interfaces/master.Validation
pnpm --filter @eulerxyz/euler-v2-sdk test— 479 tests passedpnpm --filter @eulerxyz/euler-v2-sdk typecheckpnpm --filter @eulerxyz/euler-v2-sdk buildpnpm --filter @eulerxyz/euler-v2-sdk lint— passed; existing warnings remain outside this diffgit diff --checkRelated
Summary by CodeRabbit
New Features
Bug Fixes