refactor(morpho-sdk): Extract morpho-sdk bundler action encoding#669
Conversation
5eb6445 to
fd7e814
Compare
05e19d9 to
71e4a01
Compare
fd7e814 to
b5d0ec4
Compare
33a1ca8 to
784418d
Compare
0cc8a72 to
73889ac
Compare
1bddb17 to
cef0841
Compare
Rubilmax
left a comment
There was a problem hiding this comment.
Automated /pr-review-local findings, posted as inline comments.
| Severity | Count |
|---|---|
| Critical | 1 |
| High | 4 |
| Medium | 11 |
| Low | 4 |
9 personas fired (baseline + ci-release-security). 0 agent failures. Deduplicated against branch base extract-public-reallocation (merge-base 1b736238). Scope filter dropped 2 findings on files outside the diff.
Guard against /pr-review-local's known limits: messages may drift between reruns (LLM nondeterminism); the line of a finding is always within the changed-line window of the file, but the exact line can lag the symbol by a few lines on dense files.
546d028 to
4138026
Compare
4d430a3 to
1ab3b0d
Compare
[MEDIUM] Stale line reference Suggestion: update to |
Foulks-Plb
left a comment
There was a problem hiding this comment.
Parallel PR Review (Claude) — focus: security / fund loss / tx flow
Reviewed commit: 9cb99a1b • Base: extract-public-reallocation
| Severity | Count |
|---|---|
| Critical | 0 |
| High | 0 |
| Medium | 0 |
| Low | 1 |
Transaction-flow / tx.value audit (no fund-loss bugs found)
The core refactor — moving tx.value from explicit callsite overrides (value: nativeAmount, value: reallocationFee, value: nativeAmount + reallocationFee) to a derived value computed by the new BundlerAction.encodeBundle — was traced end-to-end:
nativeTransfer(bundler3, generalAdapter1, X)emits one bundler call withvalue=X;consumeCallValuedeficits the bundler-available balance and rolls the shortfall intotx.value. ✓nativeTransfer(user, bundler3, X)enters the prefund branch (addBundlerPrefund), which increments BOTHstate.valueandavailableBundlerValue, so later value-carrying calls (e.g.reallocateTo) consume the prefund before adding totx.value. ✓reallocateToaction carriesvalue: fee; eachreallocateTocorrectly bumpstx.valuebyfeeminus any remaining prefund. ✓- Per-builder traces match the legacy explicit values:
supplyCollateral(native):tx.value = nativeAmount✓borrow(with reallocations):tx.value = Σ fees✓supplyCollateralBorrow(native + reallocations):tx.value = nativeAmount + Σ fees✓repay/repayWithdrawCollateral:tx.value = 0n✓vaultV1/vaultV2 deposit(native):tx.value = nativeAmount✓
- Callback actions inside
morphoSupplyCollateral/morphoRepayare correctly recursed inencodeBundleAction, so avalue > 0ncallback (e.g. a transfer in a callback) correctly contributes totx.value. ✓
The parity test in bundler/actions.test.ts documents the new tx.value = 130n vs legacyTx.value = 123n for the same mixed bundle — the new encoder accounts for a reallocateTo fee whose value exceeds the remaining bundler prefund (the legacy encodeBundle did not, which is why the old callsites had to override value explicitly). Removing the explicit value override is therefore only safe because the new encoder is more accurate — both observations are true together.
Other correctness checks that came up while auditing the diff:
- The
bundler-sdk-viem=== → isAddressEqualfix (separate changeset, included in this PR) is a real bug fix — the old===onbundler3/generalAdapter1recipients would miss non-checksummed inputs and either skip value-counting or skip therecipient===bundler3short-circuit innativeTransfer. The newmorpho-sdkencoder already usesisAddressEqualeverywhere.morpho-sdkaction builders source addresses fromgetChainAddresses(always canonical), so the old===bug never affected morpho-sdk outputs, but external bundler-sdk-viem callers could have hit it. BundlerErrors.MissingSignature/UnexpectedActionare now distinct classes from the ones in@morpho-org/bundler-sdk-viem. Anyone catchingBundlerErrors.Xfrom the old bundler-sdk-viem path will not match throws from the new morpho-sdk encoder viainstanceof. The PR description notesmigration-sdk-viemintentionally keeps the bundler-sdk-viem wiring, so this is the expected break, but worth flagging in release notes / migration guide.- Note on scope: the
extract-public-reallocationbase branch (separate PR) is the one that removes the legacy 1h delay margin fromReallocationData.getMarketPublicReallocations. That is a behaviour change worth reviewing on the base PR, not this one.
Findings
See inline comment.
This is an automated parallel review. Findings list is conservative — only issues a reviewer would clearly act on in this PR.
|
One finding (Low): ABI duplication — |
* docs: add morpho-ts utility re-export plan * feat(morpho-sdk): extract public reallocation data * fix(morpho-sdk): format reallocation data * test(morpho-sdk): cover reallocation data migration * docs(morpho-sdk): document reallocation data APIs * chore(morpho-sdk): drop simulation sdk dev dependency * fix(morpho-sdk): remove reallocation delay lookahead * docs: document reallocation timestamp accrual * refactor(liquidity-sdk-viem): use morpho reallocation data * chore: satisfy biome after main rebase * fix: address CI failures after reallocation refactor * docs: remove TIB changes from implementation PR * fix: normalize reallocatable vault filters * refactor: drop holdings from reallocation data * fix: optimize public reallocation capping * test: restore reallocation simulation parity * test: fill reallocation parity gaps * fix(morpho-sdk): ignore disabled reallocation markets * chore: address reallocation review feedback * test: cover reallocation timestamp overload * docs: document reallocation fallback behavior * perf: optimize reallocation state cloning * docs: clarify public reallocation API * fix: sort reallocation withdrawals bytewise * fix: address reallocation review feedback * fix: address reallocation review feedback * fix: validate reallocation data chain * fix: add liquidity reallocation delay * test: update liquidity loader snapshots * refactor(morpho-sdk): Extract morpho-sdk bundler action encoding (#669) * refactor(liquidity-sdk-viem): use morpho reallocation data * feat(morpho-sdk): extract bundler action encoding * docs(morpho-sdk): document Permit2 expiration error * test(liquidity-sdk-viem): checksum loader owner fixture * fix(morpho-sdk): remove unused bundler signature error * fix(morpho-sdk): derive bundler transaction value * docs: document bundler encodeBundle errors * test(morpho-sdk): cover bundler value derivation * docs(morpho-sdk): clarify bundler encoding constraints * docs(morpho-sdk): document native transfer skip behavior * fix: account callback bundle value * test(liquidity-sdk-viem): update loader expectations * refactor(morpho-sdk): remove unused rewards ABI * fix: normalize reallocation allocator fees * refactor: reuse market id comparator * feat(morpho-sdk): Re-export blue SDK surfaces from morpho-sdk (#714) * refactor(liquidity-sdk-viem): use morpho reallocation data * feat(morpho-sdk): re-export blue sdk surfaces * chore: align reallocation entity exports * fix(morpho-sdk): align re-export subpaths * docs: add package deprecation migration notes * fix(morpho-sdk): preserve source augment side effects
Summary
morpho-sdkintopackages/morpho-sdk.@morpho-org/bundler-sdk-viemfrommorpho-sdkdependencies and lockfile importer data.migration-sdk-viemunchanged, including its existingbundler-sdk-viemwiring, because it is slated for deprecation.BundlerErrorsnamespace intopackages/morpho-sdk/src/types/error.tsand re-export it from the local bundler action surface.Fixes SDK-186
Validation
pnpm --filter @morpho-org/morpho-sdk exec tsc --noEmit -p tsconfig.build.esm.jsonpnpm --filter @morpho-org/morpho-sdk exec tsc --noEmit -p tsconfig.build.cjs.jsonpnpm exec vitest --root . --project morpho-sdk packages/morpho-sdk/src/actions/bundler.test.ts --runpnpm lintgit diff --check