fix(community-tax): SendFrom listed-pair Sell extra-debits beyond allowance #1228
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#1228
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
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
Community-tax
SendFromon a listed-pair Sell (SendFrom→ registered pair +Cw20HookMsg::Swap) extra-debits the owneramount + sell tax(T592-2) butdeduct_allowanceonly consumesamount. A spender approved for the declared sell size can move more of the owner’s balance than the remaining allowance.Verified on the crate path:
sell_bps = 500, declaredamount = 1_000_000→ owner debit 1_050_000 while allowance drops by 1_000_000 only.Owner-signed
Send+Swapextra-debit is covered (sell_extra_debit_on_swap_send, greedy #710). There is noSendFromallowance test incommunity-tax-tokenmultitest. This is not a pair/router FoT change and not the greedy leftover #710.Parent design remains #592 (T592-1 inbound 1:1, T592-2 extra-debit). Those tickets do not state the allowance invariant.
Repro (Given / When / Then)
Given a community-tax CW20 with
sell_bps = 500, a factory-listed pair registered viaRegisterListedPair, owner balance ≥ 1_050_000, andIncreaseAllowanceof 1_000_000 to a spender (not the owner).When the spender executes
SendFrom { owner, contract: listed_pair, amount: 1_000_000, msg: Swap { … } }.Then the execute must fail closed (insufficient allowance). Owner balance, pair credit, tax sinks, and remaining allowance are unchanged.
Given the same setup except allowance is 1_050_000.
When the spender
SendFroms declaredamount = 1_000_000to that listed pair with a Swap hook.Then owner debit is 1_050_000, pair credit is 1_000_000 (no inbound FoT, T592-1), tax 50_000 lands on configured sinks, remaining allowance is 0, and
TaxPreviewdebit matches the owner debit.Expected vs actual
SendFromTaxPreview.debit, pair-direct =amount + tax)execute_send_fromcallsdeduct_allowance(…, amount)thentax::apply_transferextra-debitsamount + taxamountstay declaredamount(T592-1, H-01)amount; the hole is owner-side allowance vs debitTransferFrom(provide / limit-adjacent)amount(T592-7)SendFromamount; allowance =amountCurrent codebase
execute_send_from(same pattern onexecute_transfer_from/execute_burn_from):deduct_allowance(storage, owner, spender, block, amount)tax::apply_transfer(…, owner, to, amount, Some(&msg))tax.rspair-direct Sell:(debit, credit) = (amount + tax, amount). Router hop Sell extra-debits authenticatedSwap.traderand debitsfrombyamountonly (T592-13).SendFrom’sfromis the owner, so a spender pulling into a listed pair is pair-direct Sell, not a router hop.execute_send(owner isinfo.sender) has no allowance step; extra-debit is the owner’s own signature.SendFromis the spender path.Crate tests:
sell_extra_debit_on_swap_send/ greedy variants assert ownerSend. NoSendFrom/IncreaseAllowancerows. Layer B-ltSendFromis tax-off (noRegisterListedPair, #623).Why this is needed
Aggregators, routers, keepers, and any CW20 spender use
SendFrom, not ownerSend. Extra-debit without charging the tax against allowance means:0after a pull that tookamount + taxfrom the owner.amount(or a max-sell that usedTaxPreview.declaredinstead ofdebit) still loses the tax slice.SendFrom”. TreatingSendFromas 1:1 would be a tax-evasion hole (spender-shapedSend).Live listed templates (11611 / 11619) inherit this until a store + migrate. This ticket is the crate + tests; mainnet migrate is a separate ops step after wasm pin (do not mix Coolify/host work here).
Constraints / guardrails
amount.amount. Receive hook amount = credit.SendFrom.TransferFrom(provide) and limitPlaceLimitOrder*Sendstay 1:1.Swap.trader. Official-router hop still extra-debits authenticated trader whenfrom == config.router.deduct_allowancefor pair-direct SellSendFrommust use economic debit (amount + tax), not declaredamount. Insufficient allowance reverts before balance moves.TaxPreviewdebit/credit/tax stay the source of truth for dApp max-sell; do not invent a second formula.Relevant files
smartcontracts/contracts/community-tax-token/src/contract.rsexecute_send_fromdeductsamountthenapply_transfersmartcontracts/contracts/community-tax-token/src/tax.rsapply_transfer/tax_previewdebit for pair-direct Sellsmartcontracts/contracts/community-tax-token/src/lib.rssmartcontracts/contracts/community-tax-token/src/multitest.rsskills/AGENTS_COMMUNITY_TAX_CW20.mdSendonly; add SendFrom debit=allowancedocs/contracts-security-audit.md/docs/contracts-terraclassic.mdDo not change pair, router, or factory wasm for this.
Recommended direction
deduct_allowanceonSendFrom(samekind/debitasapply_transfer).deduct_allowance(…, debit)wheredebit = amount + tax. Other kinds (Honest,Buyoutbound is not aSendFromfrom the trader in the usual path, Transfer tax if it can appear onSendFrom) deduct the same debitapply_transferwill take fromowner. Fail closed if allowance < debit.apply_transferas today (pair creditamount, extra-debit owner).TransferFrom1:1 +deduct_allowance(amount)— it is not a Swap Send (T592-7).amount(must revert),amount+tax(success, allowance 0), unlimited allowance, unregistered pair 1:1, spoofedtraderstill extra-debits owner, manager-directory skip does not extra-debit (allowance =amount).make verify-issue-*gate grepping the new tests. Do not weaken B-lt 1:1SendFrom.Acceptance criteria
SendFrom+Swapwith allowance == declaredamountreverts; no owner debit, no pair credit, no sink credit, allowance unchanged.TaxPreview.debitsucceeds; owner −debit, pair +amount, sinks +tax, remaining allowance 0.SendFromcannot skip sell tax (same extra-debit as ownerSendfor pair-direct).TransferFromto the listed pair (no Swap hook) remains 1:1; allowance decreases byamountonly.SendFromdebit =amount; allowance =amount.traderdoes not move the victim; owner is extra-debited (T592-13).amount(#609 / #633).sell_extra_debit_on_swap_send/ greedy #710 tests stay green.Test plan (functional paths)
SendFrom1_000_000 SwapSendFromSend1_000_000 Swap (control)SendFrom1_000_000TransferFrom1_000_000 to listed pairSendFromtrader: victimonSendFromSendFromSendFromcargo testincommunity-tax-token(and existingmake verify-issue-592/verify-issue-710if those still invoke this crate). No mainnet txs in this ticket.Attack / abuse tests
amountforces listed SellSendFrominstead ofSendto skip sell taxtraderspoof to extra-debit a third walletTransferFromused as a fake sell (no Swap hook)debit, then secondSendFrom1Do not publish live instance addresses, spend keys, or a copy-paste drain recipe against columbus-5.
Verification criteria
deduct_allowanceis left on declaredamountfor pair-direct Sell.TaxPreview.debit== owner balance delta on success path T2.make verify-issue-592(or crate test target) green; B-lt tax-offSendFromstill 1:1.First-pass model recommendation
Recommendation: grok-high
Rationale: Community-tax wasm (T592) plus CW20 allowance accounting. Founder-required surface (contracts / auth / wasm). Fix is not a local three-file UI helper:
execute_send_frommust stay aligned withapply_transfer/TaxPreview, classification (pair-direct vs router hop, directory skip, greedy serde), and live listed code_ids that need a later migrate. Wrong direction (skip tax onSendFrom, or extra-debitTransferFrom) breaks H-01 or opens tax evasion. Verify with crate multitest (T1–T10 / A1–A5), not a single snapshot. Composer’s low-risk local-edit bar does not hold.cl8y-agent-control: queued
implementjobc44127c6-0854-400e-95a0-e64a09da934b(not executed; no Hetzner VM).Merged onto
origin/mainvia #1233.make verify-issue-1228was 9/9 (crate + docs). AC matched:SendFromdeductsTaxPreview.debit;TransferFromstillamount.Leftover (not this ticket): listed community-tax wasm 11611 / 11619 still need store+migrate so live tokens pick up the allowance debit. Same leftover as #1237 token wasm. Tracked in a new ops issue. Do not overload #1232 (pair
DISCOUNT_REGISTRY/ORACLE_STATEbackfill).CI: Woodpecker did not post live statuses; merge used local gitleaks + Forgejo status
ci/woodpecker/pr/woodpecker.Follow-up ops ticket: #1246. Woodpecker enablement: #1247.
columbus-5 wasm for this ticket is not live. Ops tracker: #1246.
Live pairs are 11639 / 1.16.0 (#712). LCD
HybridSimulationbelief_price: "0"still 200 (same output as omitted belief) — #1230 / #1227 / #1231 execute/query wasm still needs a 1.17.0 store+migrate (gitCONTRACT_VERSIONis still 1.15.0). Tax listed pin is 11630 (not 11611/11619); ALPHAterra1x6e64…is 1.0.0 and needs a tax cw2 bump + CMM migrate for #1228 / #1237.Tax wasm stored and listed as 11666 (tx
5A45A84A…, whitelistA9E19924…). Live ALPHA instance is still 11630 / 1.0.0 (CMM admin; no treasury migrate execute). Instance leftover #1250 + ustr-cmm #43. Trading is not frozen: 11630 stays on the whitelist and pair pins still match.