Show factory-sourced pair contracts in swap signing modal (#449) #991
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!991
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "fix/449-swap-signing-pair-contract"
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
Addresses #449 (SEC-I01 H07 / SEC-I02): the swap pre-sign summary now shows factory-sourced pair contract address(es) (truncated, copy + explorer link) so users can verify hop targets before signing.
/tradeMarket tab.resolveSwapRoutePairAddresses.ts(reused by pause/blacklist probe on Swap).Docs/skills updated:
docs/frontend.md,docs/security-model.md,skills/AGENTS_FRONTEND_SWAP_SIGNING_CONFIRMATION.md.Acceptance checklist
cd frontend-dapp && npm run test:run -- src/components/swap/__tests__/SwapPreSubmitSummary.test.tsx—renders factory-sourced pair contract addressrenders hop pair contract addresses for multihop routesdirectPairfrom LCDgetAllPairsPaginated, multihop viaROUTER_CONTRACT_ADDRESS); no indexer pair addr in submit payloadSwapPreSubmitSummary.test.tsx+resolveSwapRoutePairAddresses.test.ts; integration:SwapPage.test.tsxpre-sign test still passesThird-party verification
Manual (LocalTerra +
make dev):/swap, pick a CW20 pair, enter amount → confirmswap-pre-submit-summaryshows Pair contract with truncated bech32 + copy/explorer.swap-confirm-hop-pair-0, …)./trade/:pairAddrMarket tab, confirmtrade-market-pre-submit-summaryshows the same pair contract row.Blockers
None.
Severity: Medium
Issue: Pre-sign Pair contract(s) row can disagree with the on-chain submit path on
/swap.Impact:
#449/ SEC-I02 surfaces factory pair address(es) for user verification before signing. This prop is fed fromswapBlacklistProbe.pairAddresses, which resolves ops viaroute ?? nativeRouteInfo?.operations ?? simQuery.data?.indexerOperations(unchanged line 630). Submit instead prioritizessimData.indexerOperationswhenswapOpsRequireRouter(idxOps)(≥2 hops) at lines 753–772.For CW20↔CW20 the indexer hybrid quote runs first; a direct factory pair can exist while quote/submit use a 2-hop router path. BFS
routeis shortest-path (often 1-hop direct), so the modal can show a single direct Pair contract while the wallet signsexecute_swap_operationsthrough different hop pair(s).Attack path (no compromised indexer):
router_operations(default CW20 path).swapBlacklistProberesolves pair addresses from 1-hoproute.Contrast:
TradeMarketOrderPanelpassessimQuery.data?.indexerOperationsdirectly to the resolver — Swap wiring inherits the stalerouteOpsprecedence.Suggestion: Derive display ops with the same precedence as submit (prefer
indexerOperationswhen router execution applies).Security review — MR !991
Commit reviewed:
2585a1936223829fedc4cf7b447795d9cc63e121Scope: Pre-sign factory pair contract display for swap/trade (
SwapPreSubmitSummary,resolveSwapRoutePairAddresses, SwapPage + TradeMarketOrderPanel wiring), docs/skills updates.Outcome:
FINDINGS: 1medium+Findings
frontend-dapp/src/pages/SwapPage.tsx(inline onpairContractAddresseswiring)/swappre-sign Pair contract(s) can show BFSroutepair addresses while submit executes indexerrouter_operations(≥2 hops). Undermines #449 verification; no compromised indexer required when direct + multihop paths coexist.Not reported (reviewed, below threshold)
AddressRow+getExplorerAddressUrlbech32 validation; React text rendering for truncated labels.resolveSwapRoutePairAddressesfrom-token/to-token fallback: edge-case UX only; secondary to therouteOpsprecedence issue above.indexerOperationsdirectly (aligned with submit).Prior MR security-review threads: none found.
Label
block:securityapplied.Security fixes (
633628a4)/swappre-sign Pair contract(s) could show BFSroutepair addresses while submit executes indexerrouter_operations(≥2 hops)deriveSwapSubmitRouteOps()inswapRouteDisplay.tswith the same precedence asswapMutation(native wrap → indexer router ops whenswapOpsRequireRouter→ client BFS).SwapPageswapBlacklistProbenow usessimData.indexerOperationsvia that helper for pair resolution, pause, and blacklist probes. Unit test covers direct BFS vs indexer multihop precedence.All prior low+ findings addressed. Targeted tests pass (
swapRouteDisplay.test.ts,SwapPreSubmitSummary.test.tsx,resolveSwapRoutePairAddresses.test.ts,SwapPage.test.tsx,TradeMarketOrderPanel.submitSnapshot.test.tsx).added 1 commit
633628a4- Align swap pre-sign pair addresses with submit route ops (#449).Compare with previous version
Security review — MR !991
Commit reviewed:
633628a491aec140d811b9cf0862c3cd32f76eb2Scope: Pre-sign factory pair contract display (
SwapPreSubmitSummary,resolveSwapRoutePairAddresses,deriveSwapSubmitRouteOps), SwapPage + TradeMarketOrderPanel wiring, docs/skills updates.Outcome:
FINDINGS: 0medium+ — NONESecurity review: no medium+ findings on this diff.
Prior thread disposition
/swappre-sign Pair contract(s) could disagree with submit when indexer multihop beat BFS direct633628a4viaderiveSwapSubmitRouteOps()(same precedence asswapMutation);swapBlacklistProbenow usessimData.indexerOperationsthrough that helper. Discussion resolved.Reviewed (below threshold)
AddressRowrenders factory bech32 via React text nodes;getExplorerAddressUrlvalidates withisValidTerraBech32Addressbefore building explorer links.resolveSwapRoutePairAddressesfrom/to fallback: only when hop ops fail to match factory list; edge-case UX, not an attacker-controlled spoof path on the remediated wiring.indexerOperations+selectedPairresolver aligns with market submit (swapOpsRequireRouter→ router, else direct on selected pair).No new inline security threads on this revision.
resolved all threads
added 12 commits
main4dad91d8- Merge branch 'main' into fix/449-swap-signing-pair-contractCompare with previous version
Merged
mainintofix/449-swap-signing-pair-contractand resolved the single conflict inskills/AGENTS_FRONTEND_SWAP_SIGNING_CONFIRMATION.md.Resolution: kept this branch's #449 pair-contract row guidance (factory-sourced hop targets) and
main's #461 limit-order pre-sign field notes (SEC-I05).Targeted frontend tests (SwapPreSubmitSummary, resolveSwapRoutePairAddresses, SwapPage, TradeMarketOrderPanel.submitSnapshot) — 42/42 pass.
Security review — MR !991
Commit reviewed:
4dad91d80d80e5a609e91124efb4903f021706d5Scope: Pre-sign factory pair contract display (
SwapPreSubmitSummary,resolveSwapRoutePairAddresses,deriveSwapSubmitRouteOps), SwapPage + TradeMarketOrderPanel wiring, docs/skills updates. Post-merge re-review includes633628a4route-op alignment fix andmainmerge conflict resolution inskills/AGENTS_FRONTEND_SWAP_SIGNING_CONFIRMATION.md.Outcome:
FINDINGS: 0medium+ — NONESecurity review: no medium+ findings on this diff.
Prior thread disposition
/swappre-sign Pair contract(s) could show BFSroutepair addresses while submit executes indexerrouter_operations(≥2 hops)633628a4viaderiveSwapSubmitRouteOps()(same precedence asswapMutation);swapBlacklistProbeusessimData.indexerOperationsthrough that helper. Verified still present after4dad91d8merge.Reviewed (below threshold)
AddressRowrenders factory bech32 via React text nodes;getExplorerAddressUrlvalidates withisValidTerraBech32Addressbefore building explorer links (terraExplorer.ts).resolveSwapRoutePairAddressesfrom/to fallback: only when hop ops fail to match the LCD factory list; edge-case UX on a trusted chain source, not an attacker-controlled spoof path on the remediated wiring.indexerOperations+selectedPairresolver aligns with market submit (swapOpsRequireRouter→ router, else direct onselectedPair).swapBlacklistProbe.pairAddressesas the pre-sign row — no new bypass introduced.No new inline security threads on this revision.
mentioned in commit
37b005bd6a