No reviewers
Labels
No labels
agent:fix_bugfix
agent:fix_conflicts
agent:fix_security
agent:gap_analysis
agent:implement
agent:implement
agent:implement
agent:open_issues
agent:ready
agent:research
agent:security_audit
agent:verify
architecture
backend
blocker:hybrid
blocker:launch
blocker:limit-orders
blocker:v2
block:log_only
block:security
bug
ci
contracts
correctness
deploy
dev
devops
docs
documentation
duplicate
e2e
enhancement
epic
feature
frontend
functional-completion
gas
good first issue
governance
help wanted
high-risk
hooks
hybrid
indexer
infra
infrastructure
integrators
invalid
launch-blocker
limit-orders
localnet
localterra
low priority
missing-implementation
needs-design
ops
performance
priority
high
priority
medium
product
qa
QA
question
ready
ready
research
scripts
security
security-hardening
smartcontracts
tech-debt
testing
ux
UX
v2
verification
wontfix
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
code/cl8y-dex-terraclassic!1198
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/708-greedy-book-first"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
max_maker_fills+ optionalbook_start_hint(GreedySwapParams), not a Pattern C split.hybrid: Nonestays pool-only (G1).HybridSimulation+greedy) uses the sameresolve_swap_hybrid_modeas execute on both fields (G7 / G11 / #709). Official dApp stays onGET /route/solve.greedy_stop=remainder_to_poolafter a real book fill + pool remainder, pool-spot overflow is Skip not a VM panic (A7), plus tax/pause/blacklist/AfterSwap coverage (#710).Closes #708
Closes #709
Closes #710
Design notes
greedy) so declaredHybridSwapParamscannot deserialize as greedy (G11 / A10).Decimal::checked_from_ratio(overflow → Skip).greedy_stop:worse_than_pool|max_makers|scan_cap|empty(no maker filled) |filled|remainder_to_pool(makers filled, leftover → AMM).Invariants (G1–G14)
hybrid: None+greedy: Noneis pool-onlypool_input/book_inputon greedy/route/solve, ≥ pool-only when the book is strictly bettermax_maker_fills == 0rejects; oversize clamps to 100resolve_swap_hybrid_mode(hybrid, greedy)belief_priceormin_returnremainder_to_poolmatch_bids/match_asks/simulate_match_*GreedySwapParams; both hybrid+greedy rejected on pair query, pair execute, router sim, router executegetGasLimitForTx+ swarmgas.tsmap greedy (hook string staysswap)greedy_stop; existing Pattern C tests stay greenLeftovers #709 (correctness)
HybridSimulationwith bothhybridandgreedyerrors (same family as execute)greedy_stop=empty; full consume →filled; remainder →remainder_to_pool(notempty)No); pool-spot overflow → Skip, no panic (A7)make verify-issue-709Leftovers #710 (coverage)
from; router hop extra-debits original trader (T592-13)traderspoof does not steal fee tier or change tax subject (A9 / A14)make verify-issue-710Docs / skills
skills/AGENTS_GREEDY_BOOK_FIRST.md(G1–G14, #709/#710)docs/limit-orders.md,docs/integrators.md,docs/contracts-security-audit.md,docs/testing.md,docs/README.mdagent-player indexAGENTS_HYBRID_QUOTING.md,AGENTS_BOOK_MATCH_HINT_SECURITY.md,AGENTS_TERRACLASSIC_GAS.md,AGENTS_COMMUNITY_TAX_ROUTER.mdmake verify-issue-708·make verify-issue-709·make verify-issue-710Test plan
make verify-issue-709(dex-common + pair greedy +greedy_book_first_708+ docs grep + retest)make verify-issue-710(community-tax greedy + pause/L7/A14 +greedy_blacklist+ docs grep + retest)cargo test -p cl8y-dex-tests bid_and_hybrid_swap_partially_fills_book -- --test-threads=1vitest runhybridSwapGas + terraGas.greedymake verify-issue-708on a checkout withfrontend-dapp/node_modules(worktree without install fails vitest PATH; tests themselves passed)Not in this MR
/route/solveHybridReverseSimulationunchanged — pool-only / declared hybrid only)book_start_hinton-chainbook_input=1error string; G6 live same-side stale hint (should-fix from !1198 review, not #709/#710)marked this merge request as ready
added 1 commit
f0d0c2d5- Document greedy G4 (not an on-chain split solver).Compare with previous version
RECOMMEND: FIX
Reviewed !1198 (
feat/708-greedy-book-first,f0d0c2d5) as the MR that implements #708. GitLab has no MR iid 708; this is the open greedy book-first change.The opt-in contract path is real and mostly matches G1–G13 on execute: distinct
GreedySwapParams,hybrid: Nonestays pool-only, Decimal-rate G3 stop, G5 0-reject / 100-clamp, G8 floor, G9 remainder to AMM, matcher reuse, frontend/swarm hybrid gas envelope (not silent 600k). No fund-theft / privilege-escalation / prompt-injection path turned up on the new walk.Do not merge until the quote/execute mutex,
greedy_stopattr, and pool-spot panic are fixed.Closes #708also overclaims relative to the issue’s open acceptance checkboxes, owner opt-out note, and columbus-5 migrate.Must-fix (blocking)
1. Pair
HybridSimulationacceptshybrid+greedy; execute rejects (G11 / G7 / A11)query_hybrid_simulationprefers greedy and drops hybrid:simulate_hybrid_swap_with_feethen callsresolve_swap_hybrid_modewithhybrid: Nonewhenevergreedyis set. Execute usesresolve_swap_hybrid_mode(hybrid, greedy)and errors. Router simulate already rejects both (cannot set both hybrid and greedy on a hop).Same LCD JSON can quote greedy and fail on
Cw20HookMsg::Swap. Fix: callresolve_swap_hybrid_modeon the pair query with both fields; do not dummy-out hybrid.smartcontracts/contracts/pair/src/contract.rs(~2454–2460, ~2674–2679)2.
greedy_stop=emptyafter a real book fill + pool remainderGreedyStopReason::Emptyis documented as “no live same-side maker was filled.”greedy_stop_after_walkfalls through toEmptywhenmakers_used > 0, offer remains, and the book is exhausted (not worse-than-pool, not cap). That is the common success path (greedy_better_bid_then_pool_remainder). Indexers/bots keyingemptyas “no book contact” will mis-read fills.Add a distinct reason (e.g.
remainder_to_pool/book_exhausted) or map that case to something other thanempty. Assertgreedy_stopon the remainder test.smartcontracts/contracts/pair/src/greedy.rs(~147–174)smartcontracts/packages/dex-common/src/pair.rs(~137–138)3.
Decimal::from_ratiocan panic on greedy pool spot (DoS, opt-in only)pool_spot_netusesDecimal::from_ratio(output_reserve, input_reserve). In cosmwasm-std 1.5 this panics whennumerator * 10^18overflowsUint128(~ratio ≳ 3.4e20). Near-drained 18-dec pools can hit that. Pool-only swaps never take this path; greedy execute and query do.constant_product_net_outalready usesUint256but is unused.Use
Decimal::checked_from_ratioand treat overflow asSkipor a contract error (fail closed, full gas panic).smartcontracts/contracts/pair/src/greedy.rs(~73–82)Should-fix (same MR if cheap)
ReverseSimulateSwapOperationsignoresgreedy(TerraSwap { .. })HybridReverseSimulationhas nogreedyfield (listed as not-in-MR — either implement or document as unsupported, not silent pool-only).book_input = 1book_input.equal_is_not_strictly_betterprice == 0→Skip, not equal-rate →No. Add a real equal-rate unit test.ask_beats_residual_poolSecurity audit
ExecuteMsg::Swapstill rejected.traderstill router-stamped only (A9 / A14).greedyis set. TerraSwap callers unchanged.greedy_wrong_side_hint_falls_back_to_head.book_input = offer.max_spreadis inert on pure-book (declared_pool_input = 0); tightmin_returnis the real floor. Residual MEV, not a new trust boundary.cost == 0skip before greedy; overflow maker skip. Residual: Decimal pool overflow (must-fix 3).deny_unknown_fields; Pattern C JSON does not decode as greedy.Cw20HookMsg::Swap+ routertrader. Extra-debit should still hit the original trader. No greedy sell test (issue path 18).RESERVES; remainder uses existing CP +new_k >= k. TWAP still before reserve change./route/solve(G4, documented). Official dApp stays on the solver.greedy_stopingest, no secret/log change. N/A for DB leaks.Not BLOCK: no validated escrow theft, maker drain below pool via greedy, or auth bypass.
#708 acceptance / verification
Issue checkboxes are still unchecked. Mapping against this diff:
hybrid: Nonenever reads the bookg1_pool_only_skips_better_bidpool_input/book_inputon greedyreturn_amount≥ pool-onlyMAX_SCAN_STEPS/ park-capNone; no stale/cancelled-idreturn_amountonly; query mutex hole; drift not documentedgreedy: Nonecompile-fixed;verify-issue-708does not run hybrid suitesgas.tswired with no greedygas.test.ts; unmapped still warn/200k fallback, not greedy-specific throwmake verify-issue-708limit_order_fill+book_return_amountAttack plan: A1, A4, A8, A10 tested. A2, A3, A5, A7, A9, A12–A14 missing. A6 partial (
price=0skip only). A11 broken by must-fix 1.Functional paths 1–18: 2, 5, 7, 9 strongest. 11, 12, 14, 16, 17, 18 missing. Router greedy hop has no contract test.
make verify-issue-708is dex-common + pairgreedy+greedy_book_first_708+ two Vitest files + docs greps. It does not runmake test-contracts, Pattern C hybrid, router, community-tax, swarm, or e2e.Locally:
cargo test -p dex-common greedy_swap(9) andcargo test -p cl8y-dex-pair greedy(4) pass. GitLab pipeline 2802987818 failed withci_quota_exceededon every job (including gitleaks) — infra, not a test signal.Gap analysis
Features vs #708
Shipped: opt-in pair/router greedy, G3 Decimal pin, wasm
greedy_stop, G13 gas, skill + ADR/docs. Honored out-of-scope: do not fliphybrid: None; do not take Swap/Trade off/route/solve; no on-chain split search; no auto hint.Deferred (MR “Not in this MR”, but issue still wants them before calling #708 done):
pair_code_idmigrate, LocalTerra canary, columbus-5 canary. Until then livegreedyfails deserialize on old pair wasm.greedy_stop/ metric “share of pair-direct txs with book leg” cannot tell greedy from Pattern C.UI/UX
No retail greedy toggle. Types + gas only. Integrators get one paragraph in
docs/integrators.md— no LCD JSON cookbook, nosimulateGreedySwapinpair.ts. Fine vs “no retail requirement”; thin for the integrators label.DRY / quality
Greedy gate copied 4× in
match_*/simulate_match_*(issue asked for a pre-pass then existing matcher — G10 reuse is good, duplication is not).constant_product_net_outis dead. PairInvalidHybridParamsDisplay drops{reason}— greedymax_maker_fills == 0shows as generic “Invalid hybrid parameters”.Gas
Mapping is conservative (always makers + pool leg + scan overhead). Correct vs 600k; not tight. No wasm-size check.
Packages
36 files. Community-tax / blacklist / migration
greedy: Noneliterals are serde/struct fallout, not tax-behavior changes. AutoLP skim stays pool-only.What a user might still expect
Copy-paste LCD execute/query; SDK helper; reverse quote;
makers_usedon sim (onlylimit_book_offer_consumed+greedy_stop); indexer attr; LocalTerra proof; dApp badge (correctly out of scope).Suggested patch order
resolve_swap_hybrid_mode(hybrid, greedy)— reject both (align with execute + router).greedy_stopenum/fallback + remainder test assert.checked_from_ratioinpool_spot_net.hybrid: null;hybrid+greedyquery error; community-tax greedy sell extra-debit; pause/blacklist; expired-prefix scan cap;return_amount ≥ pool-only; equal-rate G3 unit test.greedy(do not quote pool-only).Closes #708: owner default, pair migrate/canary, indexergreedy_stop, reverse sim if deferred.Happy-path design (G1 opt-in, G3 rate stop, G8, G11 serde shape, G13 gas) is in good shape. The mutex/attr/panic issues plus the open #708 checklist are why this is FIX, not ACCEPT.
mentioned in issue #709
mentioned in issue #710
added 1 commit
83bccc28- Fix greedy query mutex, remainder_to_pool, and leftover tests (#709 #710).Compare with previous version
changed title from
feat: opt-in greedy book-first swap (#708)tofeat: opt-in greedy book-first swap (#708, leftovers #709 #710)changed the description
RECOMMEND: ACCEPT
Reviewed !1198 (
feat/708-greedy-book-first,83bccc28) as the MR that implements #708 plus leftovers #709 / #710. The prior !1198 review (RECOMMEND: FIXonf0d0c2d5) listed three merge blockers; those are fixed in this HEAD. Opt-in greedy book-first is real, mutex-aligned on quote and execute, and covered by pause / blacklist / tax / AfterSwap tests. No fund-theft, privilege-escalation, or prompt-injection path turned up.This is not a “flip default to greedy” or columbus-5 pair migrate. Official dApp stays on
GET /route/solve. Remaining items below are should-fix / follow-up, not merge blockers.Prior must-fixes (status)
HybridSimulationacceptedhybrid+greedy; execute rejected (G11 / A11)simulate_hybrid_swap_with_feecallsresolve_swap_hybrid_mode(hybrid, greedy)on both fields (pair/src/contract.rs). Tests:greedy_query_both_hybrid_and_greedy_errors,greedy_and_hybrid_mutex,greedy_router_sim_both_hybrid_and_greedy_errors.greedy_stop=emptyafter a real book fill + pool remainderGreedyStopReason::RemainderToPool/ wireremainder_to_pool. Test:greedy_better_bid_then_pool_remainder.Decimal::from_ratiopanic on greedy pool spot (A7)pool_spot_netuseschecked_from_ratio; overflow →Skip. Test:pool_spot_overflow_is_skip_not_panic. Deadconstant_product_net_outis gone.Ran on this checkout (all green):
cargo test -p dex-common greedy_swap— 9 passedcargo test -p cl8y-dex-pair greedy— 9 passedcargo test -p cl8y-dex-tests greedy_book_first_708— 21 passedcargo test -p cl8y-dex-tests greedy_blacklist— 2 passedcargo test -p cl8y-community-tax-token greedy— 3 passedFrontend greedy vitest was not re-run here (this worktree has no
frontend-dapp/node_modules). Tests exist:terraGas.greedy.test.ts,hybridSwapGas.test.ts.Security audit
ExecuteMsg::Swapstill rejected.traderstill router-stamped only (A9 / A14). Pair-direct spoof tested (greedy_pair_direct_trader_spoof_does_not_steal_fee_tier, taxgreedy_pair_direct_trader_spoof_extra_debits_from).hybrid: None+greedy: None→PoolOnly. TerraSwap callers unchanged. Test:g1_pool_only_skips_better_bid.greedy_wrong_side_hint_falls_back_to_head.MAX_SCAN_STEPS/ park 15). No greedy-specificscan_cap/ expired-head multitest (unit priority only).book_input = offer. Tests:greedy_worse_or_equal_bid_does_not_fill,greedy_mixed_prefix_stops_at_worse,greedy_does_not_match_declared_full_book_on_equal_price.RESERVESbefore the walk. Book fills do not move AMM reserves, so the pool spot is stable for the walk (G3 / G4: not a solver). G8 required.max_spreadis inert on pure-book (declared_pool_input = 0); greedy startspool_leg = 0then remainder hits the AMM, so residual pool still goes through existing CP +assert_max_spreadwhen a pool leg exists. Tightmin_returnis the real floor on a full-book consume. Residual MEV, not a new trust boundary.cost == 0skip before greedy compare. Maker overflow skip unchanged. Pool-spot overflow → Skip, no VM panic. Residual: Skip-all-makers on unpriceable pool spot can walk to scan cap then remainder to pool (fail-closed vs filling worse makers).GreedySwapParams+ tests that Pattern C JSON does not decode as greedy.greedy(quotes pool-only / declared hybrid) — documented G12, A11-adjacent footgun (see should-fix).Receive(assert_not_paused). Tests:greedy_paused_execute_rejects_then_unpause_fills;greedy_blacklist_maker_better_bid_not_filled;greedy_blacklist_taker_rejects.adversarial_tokenstillgreedy: None). Optional on #710.trader. Covered.is_swap_send_hookmatches anyCw20HookMsg::Swap. Pair-direct extra-debitsfrom; router extra-debits authenticatedtrader(T592-13). Tests:greedy_sell_extra_debit_on_swap_send,greedy_router_sell_extra_debits_authenticated_trader. No greedy buy analogue (listed as not-in-MR).RESERVES; remainder uses existing CP +new_k >= k. TWAP still before reserve change. AfterSwapreturn_asset= book net + pool net (wasm attrs + treasury; not a hook-mock)./route/solve(G4, documented). Official dApp stays on the solver.greedy_stopis not ingested. No new SQL, migrations, or secret/log surface. Unknown wasm attrs ignored. Not a leak; bots must parse raw events if they want stop reasons.Not BLOCK: no validated escrow theft, maker drain below pool via greedy, tax-subject = router, pause bypass, or auth bypass.
#708 / #709 / #710 acceptance
GitLab issue checkboxes are still unchecked. Mapping against this diff + tests:
#708
hybrid: Nonenever reads the bookpool_input/book_inputon greedyreturn_amount≥ pool-onlyremainder_to_pooltested; no ≥ pool-only control, k, or treasury = pool + book half as a dedicated assert (AfterSwap/treasury covers the split loosely)MAX_SCAN_STEPS/ park-cap multitestNone; no stale/cancelled-idgreedy_stop; drift is L8/hybrid docs, not a greedy expired-prefix testgreedy_requires_slippage_floor)pattern_c_sim_omits_greedy_stop; mutex tests)gas.ts; unmapped throw is the existing #475 path, not a greedy-specific throwmake verify-issue-708Attack plan: A1, A4, A8, A10, A11 (forward), A12, A14 tested. A2, A3, A13 missing on greedy. A5 residual by design. A7 unit-only. A11 reverse still silent.
Functional paths 1–18: 1–9, 14–16, 18 strongest. 10 partial, 11 / 12 / 17 thin (no plain router execute greedy-vs-null hop; tax router covers execute tax only).
#709
Query both-fields error, remainder_to_pool, filled, empty, worse_than_pool, equal-rate stop, overflow Skip: Pass. Router execute mutex has no dedicated multitest (sim + pair execute covered). Overflow has no execute/query multitest on absurd reserves.
#710
Tax pair-direct + router trader, pause, blacklist maker/taker, trader spoof: Pass. AfterSwap L7: Partial (wasm
return_amount/ commissions / treasury / no park — not a hook-mock like Pattern Chybrid_hook_commission_includes_pool_and_book). G1 on tax fixture omitted-greedy: Partial (generic G1 only). A13 optional: Missing.Tick the GitLab boxes that this MR actually landed so
Closes #708/#709/#710does not auto-close tickets whose remaining rows (LocalTerra, migrate, opt-out default, reverse greedy, buy-tax analogue) are still open follow-ups.Gap analysis
Features / UX
max_maker_fills+ hint) without inventing a Pattern C split — matches the #708 design.HybridReverseSimulationhas nogreedyfield. RouterReverseSimulateSwapOperationsdiscardsgreedy(TerraSwap { hybrid, .. }) and quotes pool-only whenhybridis unset. Docs (integrators.md, skill G12) say this;docs/limit-orders.mdrouter paragraph still only describeshybridunset → pool-only. A bot that reverse-sims a greedy hop gets a number (pool-only offer), then execute greedy can oversize the offer. Inefficiency, not theft.greedy_stop=worse_than_poolcan still dump remainder to the AMM after fills — stop reason ≠ “no remainder”. Bots should keyremainder_to_poolvsworse_than_poolcarefully.greedy_stop/ book-first attribution until a follow-up parser change.Should-fix (cheap; not blocking)
greedy.is_some()book_input = 1validate_greedy_hop_execute_slippage_floor→ “Hybrid swap with book_input 1 requires per-hop min_return”. Greedy hops have nobook_input. Pair path uses realinput_amount.limit-orders.mdrouter reverse paragraphreturn ≥ pool-onlyon remainder testscan_cap/ live-stale hint / router execute hopNice-to-have
greedy_poolmatch arms inorderbook.rs.gas.tsunit test (mapping is present).greedy_stop(optional per #708).Testing / e2e
make verify-issue-708/709/710(docs grep + cargo; no LocalTerra).Packages / DRY / readability / gas / best practices
match_bids/match_asks/simulate_match_*(G10) — right CosmWasm approach vs a second walker.checked_*on pool spot.fee_keepstill usesfrom_ratiofor bps ∈ [0, 10000] — safe.greedy_stop(skip_serializing_if) — G14.SWAP_STATEthen validates greedy; CosmWasm atomicity rolls back on error (hop N validates before save — slightly cleaner).What a user might expect that is missing
/route/solve).book_start_hint(forbidden by G6).Verdict
Merge-ready for the opt-in wasm + tests + docs scope. Prior quote/execute mutex,
emptystop-reason, and pool-spot panic are gone. Tick issue checkboxes (or leave LocalTerra / migrate / opt-out / reverse-reject as open children) so GitLab auto-close matches reality. Land reverse-sim reject + G8 error copy in a small follow-up if not squeezed in here.added 5 commits
main56e4c4f5- Merge origin/main into feat/708-greedy-book-first.Compare with previous version
mentioned in commit
f54de4fd4bmentioned in issue #712
mentioned in issue #708