fix: recompute reservation amounts from miner rate at reserve time#337
Open
anderdc wants to merge 2 commits into
Open
fix: recompute reservation amounts from miner rate at reserve time#337anderdc wants to merge 2 commits into
anderdc wants to merge 2 commits into
Conversation
handle_swap_reserve trusted the user-submitted to_amount/tao_amount without checking them against the miner's commitment rate. A CLI quote computed against a momentarily-bad or stale rate would get locked into the reservation, since the reservation pins amounts but not the rate. Recompute to_amount/tao_amount from the commitment rate read at reserve time and reject the request when the user-submitted values don't match, with a "rate moved — re-quote" reason. The CLI rejection translator gains a matching rate_moved rule so the user is told to re-quote. Surfaced while diagnosing swap 79.
Replace strict equality on to_amount with a one-sided slippage band so minor rate ticks between the user's CLI quote and vote_reserve no longer bounce honest reservations. The band is configurable via --slippage <percent> on `alw swap now` (default 2%) and transmitted in a new SwapReserveSynapse.slippage_bps field. Validators also enforce an internal-consistency check on tao_amount before the slippage gate.
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
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.
Swap 79 — what spawned this
Diagnosing swap 79 surfaced that a swap's reservation pins the amounts
(
to_amount/tao_amount/from_amount) but not the miner's rate.The reservation hash and stored
Reservationstruct commit the validators tospecific amounts, yet nothing tied those amounts back to the rate they were
derived from — so a quote computed against a stale rate could be locked in.
The issue
Stale-quote bounce (swap 79 and every honest user after).
handle_swap_reservetrusted the user-submitted
to_amount/tao_amountstraight from the synapse.It checked that the miner quoted a non-zero rate for the direction but never
recomputed the amounts from that rate. Miners update rates often; the rate ticks
between the user's CLI quote and
vote_reserve, so nearly every honestreservation bounced under the strict-equality approach in the previous version
of this PR.
The fix — slippage band (this PR)
Reserve recompute with one-sided slippage.
handle_swap_reservenow:to_amountfrom the commitment rate read at reservetime (
recompute_reserve_amountshelper, unchanged from the earlier version).tao_amountinternal consistency as pure arithmetic (no rate):rejects if
synapse.tao_amount != derive_tao_leg(...).quote_within_slippage:recomputed * 10_000 >= quoted * (10_000 - slippage_bps).User-settable via
--slippage <percent>onalw swap now(default 2%,transmitted as
synapse.slippage_bps; old clients that omit the field defaultto
RESERVE_SLIPPAGE_DEFAULT_BPS = 200). Validators clamp toRESERVE_SLIPPAGE_MAX_BPS = 100_000(1000%) as an integer/typo guard.The CLI shows the slippage honestly as a reserve-acceptance threshold — not
a settlement guarantee (rate pinning at settlement is a separate unshipped
change):
Rejection message and CLI translator: the
rate_movedrule invalidator_rejections.pynow matches'quoted amount is below your slippage band'.Review notes
quote_within_slippageis pure integer math —every validator evaluates the same inputs and reaches the same decision.
slippage_bpsis user-submitted but clamped on every validator before use,so a malicious value cannot cause divergence.
synapse.to_amount/synapse.tao_amount—the recomputed value is not substituted. Consensus relies on every validator
using the user's submitted values in
vote_reserve.vote_reserve— no partial on-chain vote on a rejectedsynapse.
TestReserveRateRecompute); full suite 480/482green (2 pre-existing failures unrelated to this PR — missing
embitmodule).Follow-up — initiate pinning (Part 2, still deferred)
Pinning the miner's commitment (rate + addresses) as of the reserve block is not
included; it needs a small contract change to add
reserved_blockto theReservationstruct soRis a deterministic on-chain read. Designed separatelyto avoid improvising on the consensus path.