improve(Disputer): configurable bond balance trigger/target for auto-wrap#3539
improve(Disputer): configurable bond balance trigger/target for auto-wrap#3539droplet-rl wants to merge 4 commits into
Conversation
…wrap The disputer watchdog previously wrapped native token into the HubPool bond token only when the balance fell below a hardcoded 4x bond amount (~1.8 ABT on mainnet), topping up only to that same level. This sat below the balance monitor's warn threshold (14 ABT), so low-balance alerts could never self-heal, and the balance parked exactly at the trigger so every posted bond caused another wrap. - Add BOND_BALANCE_TRIGGER / BOND_BALANCE_TARGET (whole bond tokens) to DataworkerConfig, threaded into Disputer via a new bondReserve param. Unset values retain the existing multiplier-based defaults. - Top up to the target level rather than the trigger (the target multiplier was previously only used for the allowance check). - Wrap partially when the native balance cannot cover the full shortfall, keeping a 0.5 native-token reserve for gas, instead of wrapping nothing. The insufficient-balance throw still fires when even the partial wrap cannot cover a single bond. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
droplet-rl
requested review from
bmzig,
dijanin-brat,
mrice32,
nicholaspai and
pxrl
as code owners
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5170b6a746
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const minBondAmount = bondReserve.trigger ?? bondAmount.mul(this.bondMultiplier.min); | ||
| let targetBondAmount = bondReserve.target ?? bondAmount.mul(this.bondMultiplier.target); |
There was a problem hiding this comment.
When BOND_BALANCE_TRIGGER/BOND_BALANCE_TARGET are configured below the current HubPool bondAmount, this treats the lower configured value as the minimum/allowance target. In that environment validate() can no-op with a balance or allowance below the bondAmount that HubPool.disputeRootBundle() later pulls with safeTransferFrom, so the watchdog reaches the dispute path but the dispute transaction reverts. Please reject configured levels below bondAmount or floor both thresholds at bondAmount.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — fixed in dc3ec58. validate() now floors both the trigger and target at the HubPool bondAmount (with a warning when the configured trigger is below it), so the maintained balance/allowance always covers the safeTransferFrom in disputeRootBundle(). Flooring at runtime rather than rejecting at config parse time was chosen because bondAmount is only known after the on-chain init query and can change via HubPool.setBond(), which would otherwise turn a valid config into a crash loop. Added a regression test configuring both levels below the bond amount.
Per review: the watchdog should stay lean. The configured trigger/target top-up now runs in runDataworker() when DISPUTER_ENABLED and BOND_BALANCE_TRIGGER are set (opt-in, so other DISPUTER_ENABLED deployments are unaffected), ahead of pending root bundle validation. A top-up failure logs an error but doesn't block the run. The watchdog retains its pre-existing last-resort defaults (multiples of the bond amount). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
disputeRootBundle() pulls bondAmount from the disputer, so a configured trigger/target below it would let validate() pass while the dispute itself reverts. Floor both levels at bondAmount at runtime (it is only known post-init and can change via HubPool.setBond()) and warn when the configured trigger is below it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Addressed the Codex P2 finding in dc3ec58: Note: this was rebased onto 5eb76e9 ("move bond maintenance from watchdog to disputer flow"), which landed on the branch mid-run; doc conflicts were resolved in favor of that commit's wording. The floor applies in @codex review — the updated diff floors bond reserve levels at the HubPool bond amount per your feedback. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dc3ec58e49
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const nativeBalance = await this.provider.getBalance(await this.signer.getAddress()); | ||
| if (nativeBalance.gt(mintAmount)) { | ||
| const shortfall = targetBondAmount.sub(balance); | ||
| const available = nativeBalance.sub(this.nativeGasReserve); |
There was a problem hiding this comment.
When the signer has less than the 0.5 native-token reserve and its bond-token balance is below the trigger (for example, 2x bond with the watchdog's 4x trigger), nativeBalance.sub(this.nativeGasReserve) underflows and throws before the intended insufficient-native handling. runDisputerWatchdog() calls validate() before submitting a dispute, so a signer that still has enough bond tokens for one dispute can abort solely because it cannot top up to the target; clamp the available native amount to zero when it is below the reserve.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Partially correct — the abort you describe doesn't actually occur: BigNumber here is ethers' signed @ethersproject/bignumber, so nativeBalance.sub(nativeGasReserve) returns a negative value rather than throwing. The negative available then fails the gt(bnZero) guard (no mint) and falls into the intended insufficient-native handling, which only throws when the bond balance can't cover even one dispute. I added a regression test for your exact scenario (native below the 0.5 reserve, bond balance at 2x bond vs the 4x default trigger) and it passes on the unmodified code — validate() warns and proceeds.
That said, relying on signed-arithmetic semantics for correctness is fragile, so I applied the clamp anyway in d3e4094: available is now floored at zero via bnMax, which also made the minted ternary redundant. Test suite, tsc, and eslint all pass.
…ping The negative-available case was already handled correctly (ethers BigNumber is signed, and the gt(bnZero) guards routed a negative mintAmount into the insufficient-native warn path), but the clamp makes that explicit rather than a property of signed arithmetic, and lets the redundant minted ternary go. Regression test covers a native balance below the gas reserve with a bond balance still sufficient for one dispute. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex review — d3e4094 clamps the available native amount at zero before wrapping (via |
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Codex re-review of d3e4094 came back clean. Both findings from the earlier rounds are resolved: configured trigger/target levels are floored at the HubPool bond amount in |
1 participant
Motivation
The dataworker's mainnet ABT balance monitor (warn 14 / error 2) fires long before the auto-wrap kicks in: the wrap trigger was hardcoded at 4x the HubPool bond amount (~1.8 ABT today) in the disputer watchdog, so low-balance alerts could never self-heal. Slack context: https://umaproject.slack.com/archives/C02SXLDUYFQ/p1782981089847529
Changes
BOND_BALANCE_TRIGGER/BOND_BALANCE_TARGET(whole bond tokens, e.g.15/20) onDataworkerConfig. When set andDISPUTER_ENABLED,runDataworker()runsDisputer.validate()with these levels ahead of pending root bundle validation — i.e. the disputer owns routine bond maintenance, keeping the watchdog lean. Opt-in: deployments without the env vars (e.g. the arweave persister, which also setsDISPUTER_ENABLED) are unaffected. A top-up failure logs an error without blocking the run.Disputer.validate()improvements (shared by both callers): top up to the target rather than re-parking at the trigger (the target multiplier was previously only used for the allowance check), and wrap partially when the native balance can't cover the full shortfall (keeping a 0.5 native reserve for gas) instead of wrapping nothing. The insufficient-balance throw is preserved for the case where even the partial wrap can't cover a single bond.src/dataworker/README.md.Config follow-up
Companion bot-configs PR sets trigger 15 / target 20 on
zion-across-disputer, just above the monitor's warn threshold of 14. The env vars are inert until this change is deployed.Testing
yarn test test/disputer.watchdog.ts— 7 passing (3 new cases: default top-up-to-target, configured trigger/target, partial wrap under low native balance).🤖 Generated with Claude Code