Add packages/tooling with DTF rebalance validation - #31
Open
lcamargof wants to merge 2 commits into
Open
Conversation
Disaster-first validator for Index DTF rebalance proposals, built on the local SDK and DTF catalog, plus the skill for acting on its output. Co-Authored-By: luis.camargo@reserve.org <luis.camargo@reserve.org>
Contributor
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
📝 WalkthroughWalkthroughAdded a private tooling package and CLI for validating DTF rebalance proposals. The validator decodes calldata, reconstructs SDK outputs, checks governance and market data, reports failures and warnings, and prompts for manual review. ChangesDTF rebalance validation
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 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 |
Co-Authored-By: luis.camargo@reserve.org <luis.camargo@reserve.org>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 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 `@docs/wiki/domains/tooling.md`:
- Around line 24-25: Update the SDK boundary statement in the tooling
documentation to say that only protocol reads and canonical calldata
construction go through the SDK. Remove the claims that decoding and math are
SDK responsibilities, while preserving the existing explanation of locally owned
review logic and sources.
In `@packages/tooling/skills/validating-dtf-rebalances/SKILL.md`:
- Around line 35-41: Add an explicit turnover-warning case to Step 3 alongside
the existing WARN ownership mappings, naming the responsible owner and the
decision they must make so every turnover WARN receives an owner and resolution
path.
In `@packages/tooling/src/rebalance-validation/checks/library.ts`:
- Around line 75-110: Update checkLibraryReproduction to validate the complete
startRebalance payload, including index-aligned token addresses,
maxAuctionSizesUsd ordering, rebalance.target, auctionLauncherWindow, and ttl,
rather than only derived weights and prices. Compare decoded fields or encoded
calldata using the existing proposal and derived values, report every mismatch
through the existing validation flow, and revise the success message so it
states that the full payload reproduces correctly.
In `@packages/tooling/src/rebalance-validation/checks/liquidity.ts`:
- Around line 128-142: Update the liquidity asset filtering near the `flagged`
calculation to distinguish weak routes from unavailable liquidity: use the SDK’s
typed `liquidity.level` values rather than a hardcoded list, and handle
`"error"`, `"unknown"`, and `"failed"` as route-unavailable outcomes. Preserve
`"low"` and `"insufficient"` as weak-route flags, while allowing `"medium"` and
`"high"` to remain valid without being dropped.
In `@packages/tooling/src/rebalance-validation/cli.ts`:
- Around line 7-25: Update main to use process.exitCode instead of
process.exit() for both the missing-input error path and the final validation
result, then return naturally so console output can flush before Node exits.
In `@packages/tooling/src/rebalance-validation/context.ts`:
- Line 66: Normalize each token address with getAddress when constructing
currentBalances in the rebalance validation context, so its keys match the
checksummed addresses produced by decodeStartRebalance. Preserve the existing
balance fallback and Map construction behavior.
In `@packages/tooling/src/rebalance-validation/validate.ts`:
- Around line 79-86: Update nativePrice to record a warn on the report when no
wrapped native token is found in context.tokens, before returning the existing 0
fallback. Reuse the report object and warning pattern established by the other
soft-failure paths in this file, while preserving the current price lookup
behavior when wrapped is present.
- Around line 56-60: Update the fetchPoolQuotes call in
validateRebalanceProposal’s Promise.all to catch failures and return the empty
fallback matching fetchPoolQuotes’s actual return type, verifying its definition
in pool-prices.ts rather than assuming Map. Preserve the existing fallbacks for
fetchListedCoinsByAddress and fetchPreviousRebalance so validation continues and
the report is printed when pool-price retrieval fails.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 7fcc12fd-7c6a-4b9b-9893-bb72f631bc6f
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (25)
docs/wiki/domains/tooling.mddocs/wiki/index.mddocs/wiki/log.mddocs/wiki/progress.mdpackages/tooling/README.mdpackages/tooling/package.jsonpackages/tooling/skills/validating-dtf-rebalances/SKILL.mdpackages/tooling/src/rebalance-validation/checks/basket.tspackages/tooling/src/rebalance-validation/checks/governance.tspackages/tooling/src/rebalance-validation/checks/identity.tspackages/tooling/src/rebalance-validation/checks/library.tspackages/tooling/src/rebalance-validation/checks/liquidity.tspackages/tooling/src/rebalance-validation/checks/prices.tspackages/tooling/src/rebalance-validation/cli.tspackages/tooling/src/rebalance-validation/context.tspackages/tooling/src/rebalance-validation/human-review.tspackages/tooling/src/rebalance-validation/proposal-url.tspackages/tooling/src/rebalance-validation/report.tspackages/tooling/src/rebalance-validation/sources/pool-prices.tspackages/tooling/src/rebalance-validation/sources/token-identity.tspackages/tooling/src/rebalance-validation/start-rebalance.tspackages/tooling/src/rebalance-validation/validate.tspackages/tooling/tests/rebalance-validation.test.tspackages/tooling/tsconfig.jsonpackages/tooling/vitest.config.ts
Comment on lines
+24
to
+25
| - Protocol reads, decoding and math go through the SDK. The package holds only the | ||
| review logic and the sources the SDK deliberately does not own. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the SDK boundary statement.
Line [24] says that decoding and math go through the SDK. The validator performs local decoding in recoverTokenInputs and local review math in ordersOfMagnitude and bandUsage. Narrow this statement to protocol reads and canonical calldata construction.
Suggested wording
- Protocol reads, decoding and math go through the SDK. The package holds only the
- review logic and the sources the SDK deliberately does not own.
+ Protocol reads and canonical calldata construction go through the SDK. Validator-
+ specific decoding and review math stay in this package, together with sources the
+ SDK deliberately does not own.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - Protocol reads, decoding and math go through the SDK. The package holds only the | |
| review logic and the sources the SDK deliberately does not own. | |
| - Protocol reads and canonical calldata construction go through the SDK. Validator- | |
| specific decoding and review math stay in this package, together with sources the | |
| SDK deliberately does not own. |
🤖 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 `@docs/wiki/domains/tooling.md` around lines 24 - 25, Update the SDK boundary
statement in the tooling documentation to say that only protocol reads and
canonical calldata construction go through the SDK. Remove the claims that
decoding and math are SDK responsibilities, while preserving the existing
explanation of locally owned review logic and sources.
Comment on lines
+35
to
+41
| 3. Take each `WARN` to the person who owns it. Done when each warning has a | ||
| named owner and an answer: | ||
| - trade above $10,000, price impact above 5%, or liquidity below $50,000 → | ||
| the trading desk, who decides whether to buy inventory before the auction. | ||
| - basket addition or removal → whoever owns the index mandate. | ||
| - permissionless tail (`ttl` beyond the launcher window) → the auction | ||
| launcher operator. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Assign an owner for turnover warnings.
The outcomes pass includes turnover, but this section assigns owners for trade size, price impact, liquidity, basket changes, and TTL only. Add an explicit owner and decision path for turnover warnings. Otherwise Step 3 can leave a WARN without a named owner.
🤖 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/tooling/skills/validating-dtf-rebalances/SKILL.md` around lines 35 -
41, Add an explicit turnover-warning case to Step 3 alongside the existing WARN
ownership mappings, naming the responsible owner and the decision they must make
so every turnover WARN receives an owner and resolution path.
Comment on lines
+75
to
+110
| const mismatches = rebalance.tokens.flatMap((token, index) => { | ||
| const expected = derived.tokens[index]; | ||
| if (!expected) return [`${tokens[index]!.symbol}: missing from re-derivation`]; | ||
| const worst = Math.max( | ||
| drift(token.weight.low, expected.weight.low), | ||
| drift(token.weight.spot, expected.weight.spot), | ||
| drift(token.weight.high, expected.weight.high), | ||
| drift(token.price.low, expected.price.low), | ||
| drift(token.price.high, expected.price.high), | ||
| ); | ||
|
|
||
| return worst > WEIGHT_TOLERANCE ? [`${tokens[index]!.symbol}: ranges drift by ${formatPercent(worst, 5)}`] : []; | ||
| }); | ||
|
|
||
| report.record( | ||
| "disasters", | ||
| mismatches.length === 0 ? "pass" : "fail", | ||
| mismatches.length === 0 | ||
| ? "weights and price ranges reproduce from the rebalance library" | ||
| : "encoded ranges do not reproduce from the rebalance library", | ||
| mismatches.join(" · ") || `${rebalance.tokens.length} tokens · price errors ${describeErrors(basket)}`, | ||
| ); | ||
|
|
||
| const limitDrift = Math.max( | ||
| drift(rebalance.limits.low, derived.limits.low), | ||
| drift(rebalance.limits.spot, derived.limits.spot), | ||
| drift(rebalance.limits.high, derived.limits.high), | ||
| ); | ||
| report.record( | ||
| "disasters", | ||
| limitDrift <= WEIGHT_TOLERANCE ? "pass" : "fail", | ||
| limitDrift <= WEIGHT_TOLERANCE | ||
| ? "rebalance limits reproduce from the rebalance library" | ||
| : "rebalance limits do not reproduce from the rebalance library", | ||
| `encoded high ${Number(rebalance.limits.high) / 1e18} vs derived ${Number(derived.limits.high) / 1e18}`, | ||
| ); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Show the parameter and return shape of the SDK start-rebalance builder.
set -euo pipefail
rg -n -C 30 'buildIndexDtfStartRebalanceArgs' packages/sdk/src | head -n 250Repository: reserve-protocol/dtf-interface
Length of output: 6543
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the rebalance validation check and the SDK start-rebalance builder implementation.
printf '== relevant files ==\n'
fd -a 'library\.ts$|buildStartRebalanceArgs|index-dtf' packages/tooling packages/sdk | head -n 100
printf '\n== library.ts outline/contents ==\n'
wc -l packages/tooling/src/rebalance-validation/checks/library.ts
sed -n '1,150p' packages/tooling/src/rebalance-validation/checks/library.ts
printf '\n== builder files matching start rebalance args ==\n'
rg -n -C 40 'buildStartRebalanceArgs|type .*StartRebalance|interface .*StartRebalance|startRebalance' packages/sdk/src packages/tooling/src | head -n 360Repository: reserve-protocol/dtf-interface
Length of output: 40600
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Determine whether derived.tokens comes from the SDK builder and whether it preserves raw token ordering.
printf '== call sites of buildIndexDtfStartRebalanceArgs ==\n'
rg -n -C 20 'buildIndexDtfStartRebalanceArgs' packages/tooling packages/sdk
printf '\n== possible encoded rebalance payload consumers ==\n'
rg -n -C 20 'buildIndexDtfStartRebalance|buildStartRebalanceArgs|rebalanceLiquidityTrades|tokens.*symbol|address' packages/tooling/src packages/sdk/src | head -n 400Repository: reserve-protocol/dtf-interface
Length of output: 47908
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '== index-dtf basket files ==\n'
fd -a 'index|start' packages/sdk/src/index-dtf/dtf | sort
printf '\n== buildStartRebalanceArgs and related start-rebalance helpers ==\n'
rg -n -C 45 'buildStartRebalanceArgs|StartRebalance|startRebalance|buildStartRebalance|auctionLauncherWindow|ttl|maxAuctionSize|inRebalance' packages/sdk/src/index-dtf/dtf packages/sdk/src/index-dtf | head -n 500
printf '\n== candidate implementation files ==\n'
for f in $(fd -a 'start|Rebalance|rebalance|price|weight|shares|basket|index' packages/sdk/src/index-dtf/dtf | sort); do
echo "--- $f"
wc -l "$f"
doneRepository: reserve-protocol/dtf-interface
Length of output: 42602
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '-- rebalance-args'
sed -n '1,140p' packages/sdk/src/index-dtf/dtf/basket/rebalance-args.ts
printf '%s\n' '-- types'
sed -n '1,240p' packages/sdk/src/index-dtf/dtf/basket/types.ts
printf '%s\n' '-- start-rebalance'
fd -a 'start-rebalance\.ts$' packages/sdk/src | while read -r f; do
echo "--- $f"
sed -n '1,200p' "$f"
done
printf '%s\n' '-- package lock dtf-rebalance-lib version'
rg -n '"`@reserve-protocol/dtf-rebalance-lib`"|dtf-rebalance-lib' package.json packages/*/package.json package-lock.json pnpm-lock.yaml yarn.lock 2>/dev/null | head -n 80Repository: reserve-protocol/dtf-interface
Length of output: 11138
Compare the full startRebalance payload.
checkLibraryReproduction derives the same token order as the proposal uses, but derived.tokens[index] only has weight, price, and max auction shape, so it can never catch swapped token addresses or reordered maxAuctionSizesUsd. The check also ignores rebalance.target, auctionLauncherWindow, and ttl. Compare all decoded fields/index-aligned assets, or compare the encoded calldata directly, and update the pass message to match.
🤖 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/tooling/src/rebalance-validation/checks/library.ts` around lines 75
- 110, Update checkLibraryReproduction to validate the complete startRebalance
payload, including index-aligned token addresses, maxAuctionSizesUsd ordering,
rebalance.target, auctionLauncherWindow, and ttl, rather than only derived
weights and prices. Compare decoded fields or encoded calldata using the
existing proposal and derived values, report every mismatch through the existing
validation flow, and revise the success message so it states that the full
payload reproduces correctly.
Comment on lines
+128
to
+142
| let liquidity; | ||
| try { | ||
| liquidity = await sdk.index.getRebalanceLiquidity({ chainId: context.chainId, nativePrice, trades }); | ||
| } catch (error) { | ||
| report.record("outcomes", "warn", "liquidity route unavailable — check impact by hand", String(error)); | ||
|
|
||
| return; | ||
| } | ||
|
|
||
| const symbols = new Map(context.tokens.map((token) => [token.address.toLowerCase(), token.symbol])); | ||
| const flagged = liquidity.assets.filter( | ||
| (asset) => | ||
| Math.abs(asset.liquidity.priceImpact) > PRICE_IMPACT_FLAG_PERCENT || | ||
| ["low", "insufficient", "error", "failed", "unknown"].includes(asset.liquidity.level), | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Show the liquidity level union and the getRebalanceLiquidity response type.
set -euo pipefail
rg -n -C 15 'getRebalanceLiquidity|IndexDtfRebalanceLiquidity' packages/sdk/src | head -n 250Repository: reserve-protocol/dtf-interface
Length of output: 20079
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
fd -a 'liquidity\.ts$' packages/tooling packages/sdk | sed 's#^\./##'
echo "== tooling check file outline/lines =="
if [ -f packages/tooling/src/rebalance-validation/checks/liquidity.ts ]; then
wc -l packages/tooling/src/rebalance-validation/checks/liquidity.ts
ast-grep outline packages/tooling/src/rebalance-validation/checks/liquidity.ts --view expanded || true
sed -n '1,220p' packages/tooling/src/rebalance-validation/checks/liquidity.ts | cat -n
fi
echo "== sdk liquidity implementation outline/lines =="
sdk_files=$(fd -a 'liquidity\.ts$' packages/sdk/src | sed 's#^\./##')
for f in $sdk_files; do
if [[ "$f" != *test.ts ]]; then
echo "--- $f"
wc -l "$f"
ast-grep outline "$f" --view expanded || true
sed -n '1,260p' "$f" | cat -n
fi
done
echo "== usage of flagged level list =="
rg -n '"low"|insufficient|warning|weak|level' packages/tooling/src/rebalance-validation/checks/liquidity.ts packages/tooling package.json tsconfig*.json 2>/dev/null || trueRepository: reserve-protocol/dtf-interface
Length of output: 21391
Don’t mix weak-route flagging with route failures.
IndexDtfRebalanceLiquidityAsset.liquidity.level is part of that SDK type, but its union includes "medium" | "high" in addition to the failure levels. The current hardcoded list drops those routes if the level is renamed, and the "error" | "unknown" | "failed" values should be treated as unavailable liquidity rather than normal weak-route levels.
🤖 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/tooling/src/rebalance-validation/checks/liquidity.ts` around lines
128 - 142, Update the liquidity asset filtering near the `flagged` calculation
to distinguish weak routes from unavailable liquidity: use the SDK’s typed
`liquidity.level` values rather than a hardcoded list, and handle `"error"`,
`"unknown"`, and `"failed"` as route-unavailable outcomes. Preserve `"low"` and
`"insufficient"` as weak-route flags, while allowing `"medium"` and `"high"` to
remain valid without being dropped.
Comment on lines
+7
to
+25
| async function main(): Promise<void> { | ||
| const input = process.argv[2]; | ||
| if (!input) { | ||
| console.error("usage: pnpm validate:rebalance <governance proposal url>"); | ||
| process.exit(2); | ||
| } | ||
|
|
||
| const url = parseProposalUrl(input); | ||
| console.log(`Validating ${input}`); | ||
| const { report } = await validateRebalanceProposal(createDtfSdk(), url); | ||
| report.print(); | ||
|
|
||
| console.log("\nStill needs a human"); | ||
| for (const question of HUMAN_REVIEW_QUESTIONS) { | ||
| console.log(` - ${question}`); | ||
| } | ||
|
|
||
| process.exit(report.failures > 0 ? 1 : 0); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Use process.exitCode instead of process.exit() to avoid truncating output.
process.exit() runs right after console.error (line 11) and after report.print() plus the human-review loop (line 24). When stdout is redirected to a file or pipe (common in CI), Node's stream writes are not guaranteed to flush before process.exit() terminates the process. This risks truncating the validation report, undermining a tool whose output is the entire deliverable.
Set process.exitCode instead and let the function return naturally, so Node exits only after all output flushes.
🛡️ Proposed fix
const input = process.argv[2];
if (!input) {
console.error("usage: pnpm validate:rebalance <governance proposal url>");
- process.exit(2);
+ process.exitCode = 2;
+ return;
}
const url = parseProposalUrl(input);
console.log(`Validating ${input}`);
const { report } = await validateRebalanceProposal(createDtfSdk(), url);
report.print();
console.log("\nStill needs a human");
for (const question of HUMAN_REVIEW_QUESTIONS) {
console.log(` - ${question}`);
}
- process.exit(report.failures > 0 ? 1 : 0);
+ process.exitCode = report.failures > 0 ? 1 : 0;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| async function main(): Promise<void> { | |
| const input = process.argv[2]; | |
| if (!input) { | |
| console.error("usage: pnpm validate:rebalance <governance proposal url>"); | |
| process.exit(2); | |
| } | |
| const url = parseProposalUrl(input); | |
| console.log(`Validating ${input}`); | |
| const { report } = await validateRebalanceProposal(createDtfSdk(), url); | |
| report.print(); | |
| console.log("\nStill needs a human"); | |
| for (const question of HUMAN_REVIEW_QUESTIONS) { | |
| console.log(` - ${question}`); | |
| } | |
| process.exit(report.failures > 0 ? 1 : 0); | |
| } | |
| async function main(): Promise<void> { | |
| const input = process.argv[2]; | |
| if (!input) { | |
| console.error("usage: pnpm validate:rebalance <governance proposal url>"); | |
| process.exitCode = 2; | |
| return; | |
| } | |
| const url = parseProposalUrl(input); | |
| console.log(`Validating ${input}`); | |
| const { report } = await validateRebalanceProposal(createDtfSdk(), url); | |
| report.print(); | |
| console.log("\nStill needs a human"); | |
| for (const question of HUMAN_REVIEW_QUESTIONS) { | |
| console.log(` - ${question}`); | |
| } | |
| process.exitCode = report.failures > 0 ? 1 : 0; | |
| } |
🤖 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/tooling/src/rebalance-validation/cli.ts` around lines 7 - 25, Update
main to use process.exitCode instead of process.exit() for both the
missing-input error path and the final validation result, then return naturally
so console output can flush before Node exits.
| otherActions, | ||
| version, | ||
| supply, | ||
| currentBalances: new Map(totalAssets.tokens.map((token, index) => [token, totalAssets.balances[index] ?? 0n])), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect the return type and address casing of getTotalAssets in the SDK.
set -euo pipefail
rg -n -C 12 'getTotalAssets' packages/sdk/src | head -n 200Repository: reserve-protocol/dtf-interface
Length of output: 16573
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate relevant files =="
fd 'context\.ts|start-rebalance\.ts|dtf/index\.ts|current\.ts|types/index-dtf\.(ts|d\.ts)$' packages/sdk packages/tooling -t f | sort
echo
echo "== context.ts outline =="
ast-grep outline packages/tooling/src/rebalance-validation/context.ts --view compact || true
echo
echo "== context.ts relevant lines =="
cat -n packages/tooling/src/rebalance-validation/context.ts
echo
echo "== SDK getTotalAssets implementation =="
fd -a 'index\.ts$' packages/sdk/src/index-dtf/dtf | while read -r f; do rg -n -C 20 'export async function getTotalAssets|function getTotalAssets|getTotalAssets' "$f"; done
echo
echo "== viem getAddress usage around token decoding =="
rg -n -C 8 'getAddress\b|target|calldata|token' packages/tooling packages/sdk/src | head -n 300Repository: reserve-protocol/dtf-interface
Length of output: 4930
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== getTotalAssets files =="
rg -n -C 35 'export async function getTotalAssets|function getTotalAssets|const getTotalAssets|getTotalAssets\b' packages/sdk/src/index-dtf packages/sdk/src/types packages/sdk/src -g '*.ts' | head -n 350
echo
echo "== start-rebalance decode tokens =="
cat -n packages/tooling/src/rebalance-validation/start-rebalance.ts
echo
echo "== consumer lookups =="
rg -n -C 5 'currentBalances|rebalance\.tokens|rebalance\.targets|decoded\.tokens' packages/tooling/src/rebalance-validation -g '*.ts'Repository: reserve-protocol/dtf-interface
Length of output: 49752
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== candidate dtf files =="
fd -p 'packages/sdk/src/index-dtf/*' -t f | sort
echo
echo "== exact getTotalAssets symbols in package files =="
python3 - <<'PY'
from pathlib import Path
p=Path('packages/sdk/src/index-dtf')
for root, dirs, files in sorted(os.walk(p)):
for f in files:
if f.endswith('.ts'):
path=Path(root)/f
text=path.read_text(errors='ignore')
if 'getTotalAssets' in text:
print(f'--- {path} ---')
lines=text.splitlines()
for i,l in enumerate(lines,1):
if 'getTotalAssets' in l:
start=max(1,i-25); end=min(len(lines),i+60)
for j in range(start,end+1):
print(f'{j:4}: {lines[j-1]}')
PY
echo
echo "== all occurrences in repository =="
rg -n -C 20 '\bgetTotalAssets\b|export .*getTotalAssets|function getTotalAssets|const getTotalAssets|tokens: .*balances' packages/sdk packages/tooling packages -g '*.ts' | head -n 500Repository: reserve-protocol/dtf-interface
Length of output: 7056
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
p = Path('packages/sdk/src/index-dtf')
for root in sorted(p.rglob('')):
if not root.is_dir():
continue
for f in sorted(root.glob('*.ts')):
text = f.read_text(errors='ignore')
hits = [(i, line.strip()) for i, line in enumerate(text.splitlines(), 1) if 'getTotalAssets' in line or 'tokens:' in line or 'balances' in line]
if hits:
print(f'--- {f} ---')
for i, line in hits[:40]:
print(f'{i:4}: {line}')
PY
echo
echo "== package exports =="
rg -n -C 10 '^export {[^}]+getTotalAssets|getTotalAssets\s*\(' packages/sdk/src | head -n 200Repository: reserve-protocol/dtf-interface
Length of output: 12910
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== full getTotalAssets implementation =="
cat -n packages/sdk/src/index-dtf/dtf/index.ts | sed -n '170,220p'
echo
echo "== current bal mapper implementation =="
cat -n packages/sdk/src/index-dtf/dtf/basket/current.ts | sed -n '1,90p'
echo
echo "== map get behavior probe =="
node - <<'JS'
const raw = ["0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48"];
const checksum = "0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48";
const m = new Map(raw.map((token, index) => [token, 1_000_000n]));
console.log("raw key lookup:", m.get(raw[0]));
console.log("checksum lookup:", m.get(checksum));
console.log("has checksum:", m.has(checksum));
JSRepository: reserve-protocol/dtf-interface
Length of output: 5307
Normalize currentBalances keys with getAddress.
getTotalAssets returns the contract token addresses directly, while decodeStartRebalance normalizes rebalance tokens via getAddress. Map keys are compared by exact string, so lowercase SDK addresses miss the checksummed lookups used by the validation checks.
🐛 Proposed fix
- currentBalances: new Map(totalAssets.tokens.map((token, index) => [token, totalAssets.balances[index] ?? 0n])),
+ currentBalances: new Map(
+ totalAssets.tokens.map((token, index) => [getAddress(token), totalAssets.balances[index] ?? 0n]),
+ ),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| currentBalances: new Map(totalAssets.tokens.map((token, index) => [token, totalAssets.balances[index] ?? 0n])), | |
| currentBalances: new Map( | |
| totalAssets.tokens.map((token, index) => [getAddress(token), totalAssets.balances[index] ?? 0n]), | |
| ), |
🤖 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/tooling/src/rebalance-validation/context.ts` at line 66, Normalize
each token address with getAddress when constructing currentBalances in the
rebalance validation context, so its keys match the checksummed addresses
produced by decodeStartRebalance. Preserve the existing balance fallback and Map
construction behavior.
Comment on lines
+56
to
+60
| const [quotes, listed, previous] = await Promise.all([ | ||
| fetchPoolQuotes(context.chainId, addresses), | ||
| fetchListedCoinsByAddress(context.chainId).catch(() => new Map()), | ||
| fetchPreviousRebalance(sdk, context).catch(() => undefined), | ||
| ]); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Add a .catch() fallback to fetchPoolQuotes.
fetchListedCoinsByAddress and fetchPreviousRebalance both fall back gracefully with .catch(). fetchPoolQuotes does not. If the pool-price source fails or times out, Promise.all rejects. validateRebalanceProposal throws before report.print() runs in cli.ts. This discards the already-computed disaster-level results (governance routing, basket membership, library reproduction) with no output at all.
Add a .catch() fallback for fetchPoolQuotes, consistent with its siblings, so a single external data-source outage degrades to warnings instead of losing the whole report.
🛡️ Proposed fix
const [quotes, listed, previous] = await Promise.all([
- fetchPoolQuotes(context.chainId, addresses),
+ fetchPoolQuotes(context.chainId, addresses).catch(() => new Map()),
fetchListedCoinsByAddress(context.chainId).catch(() => new Map()),
fetchPreviousRebalance(sdk, context).catch(() => undefined),
]);Note: match the empty fallback value to fetchPoolQuotes's actual return type (a Map is assumed here based on the sibling pattern; confirm against its definition in sources/pool-prices.ts).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const [quotes, listed, previous] = await Promise.all([ | |
| fetchPoolQuotes(context.chainId, addresses), | |
| fetchListedCoinsByAddress(context.chainId).catch(() => new Map()), | |
| fetchPreviousRebalance(sdk, context).catch(() => undefined), | |
| ]); | |
| const [quotes, listed, previous] = await Promise.all([ | |
| fetchPoolQuotes(context.chainId, addresses).catch(() => new Map()), | |
| fetchListedCoinsByAddress(context.chainId).catch(() => new Map()), | |
| fetchPreviousRebalance(sdk, context).catch(() => undefined), | |
| ]); |
🤖 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/tooling/src/rebalance-validation/validate.ts` around lines 56 - 60,
Update the fetchPoolQuotes call in validateRebalanceProposal’s Promise.all to
catch failures and return the empty fallback matching fetchPoolQuotes’s actual
return type, verifying its definition in pool-prices.ts rather than assuming
Map. Preserve the existing fallbacks for fetchListedCoinsByAddress and
fetchPreviousRebalance so validation continues and the report is printed when
pool-price retrieval fails.
Comment on lines
+79
to
+86
| async function nativePrice(sdk: DtfSdk, context: ProposalContext): Promise<number> { | ||
| const symbol = NATIVE_TOKEN_BY_CHAIN[context.chainId] ?? "ETH"; | ||
| const wrapped = context.tokens.find((token) => token.symbol.toUpperCase() === `W${symbol}`); | ||
| if (!wrapped) return 0; | ||
| const [price] = await sdk.client.api.getTokenPrices({ chainId: context.chainId, addresses: [wrapped.address] }); | ||
|
|
||
| return price?.price ?? 0; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Report when the native-token price cannot be resolved.
nativePrice returns 0 when no wrapped native token is found in context.tokens, with no report entry. Every other soft-failure path in this file (line 58, line 59) records a warn when a data source is unavailable. A silent 0 here means a basket missing the expected wrapped-native token flows into checkTradeLiquidity unnoticed, instead of being surfaced for manual review.
Record a warn on the report when wrapped is not found, before falling back to 0.
🤖 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/tooling/src/rebalance-validation/validate.ts` around lines 79 - 86,
Update nativePrice to record a warn on the report when no wrapped native token
is found in context.tokens, before returning the existing 0 fallback. Reuse the
report object and warning pattern established by the other soft-failure paths in
this file, while preserving the current price lookup behavior when wrapped is
present.
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.
Summary
New private workspace package
@reserve-protocol/tooling: the home for validation scripts and agent skills that review protocol activity, built on the localsdk+dtf-catalograther than reimplementing protocol logic. First workflow is rebalance-proposal validation, from a proposal URL:pnpm --filter @reserve-protocol/tooling validate:rebalance "https://app.reserve.org/bsc/index-dtf/cmc20/governance/proposal/<id>"The review is two passes and only the first can block (nonzero exit):
disasters— the calldata moves value into the wrong place: governance routing (singlestartRebalanceon the DTF, through the governor whose timelock holdsREBALANCE_MANAGER), calldata re-derived throughbuildIndexDtfStartRebalanceArgs, held assets present in the calldata, encoded price vs pool price per asset, basket shares re-valued at pool prices, per-share units vs the last executed rebalance, and each address against a third-party address→coin map.outcomes— the trade is right but may fill badly: launcher-window/TTL tail, turnover vs AUM, legs above $10k, constituents under $50k pooled liquidity, and per-leg price impact fromPOST /rebalance/liquidity.Two non-obvious decisions:
Recovering inputs is where the encoding subtleties live (
start-rebalance.ts):skills/validating-dtf-rebalances/SKILL.mdcovers what a human still has to do: which team owns each warning class, and the questions no data source can settle (is the constituent universe official, is a wrapper canonical, is the permissionless tail intended) — printed on every run so a clean report is not mistaken for a completed review.Verified against the live CMC20 August 2026 proposal: 11 pass, 6 warn, 0 fail — SHIB addition at a $10.3k buy with ~11.6% impact, plus HBAR/SUI under the liquidity floor. Running it against real data is what caught the two unit bugs the tests could not (per-share weight scaling, and
priceImpactalready being percent).Note:
wiki-lintreports pre-existing ledger-row-length failures onmain; this branch adds no new ones.Link to Devin session: https://app.devin.ai/sessions/8046eb248e094c8c851b433f5470ee92
Requested by: @lcamargof
Summary by CodeRabbit