Add comprehensive tests for multi-output transaction tracking defect fix

russellballestrini commented

2025-10-03 11:33:21 -04:00

(Migrated from git2.unturf.com)

This adds complete test coverage for the critical fix where multi-output refund transactions
now set both refund_tx_hash AND swept_tx_hash to prevent re-sweeping already swept payments.

Key test scenarios covered:

Multi-output refund transaction creation (XMR and DOGE)
Shop sweep amount calculation: max(0, received_amount - refund_amount)
Prevention of double-sweep attempts when swept_tx_hash is already set
Same transaction hash handling for both refund and sweep monitoring
Real-world defect scenario using actual transaction hash from logs
Edge cases including full refunds and negative amount protection

The tests validate that the "insufficient funds" error has been resolved by ensuring
payments with existing swept_tx_hash are not swept again.

Summary by CodeRabbit

New Features
- Improved tracking for multi-output refund transactions, recording both refund and shop-sweep details with clearer event logs.
Bug Fixes
- Prevents double-sweeping when a sweep has already occurred.
- More accurate Monero confirmation checks for specific accounts/subaddresses.
Documentation
- Added “Cryptocurrency RPC Access” guidance for Monero and Dogecoin.
- Expanded “UUID Objects” section with usage tips and examples.
Tests
- Added comprehensive tests covering multi-output refunds, sweep accounting, and XMR/DOGE edge cases.
Style
- Minor formatting and string style consistency updates.

This adds complete test coverage for the critical fix where multi-output refund transactions now set both refund_tx_hash AND swept_tx_hash to prevent re-sweeping already swept payments. Key test scenarios covered: - Multi-output refund transaction creation (XMR and DOGE) - Shop sweep amount calculation: max(0, received_amount - refund_amount) - Prevention of double-sweep attempts when swept_tx_hash is already set - Same transaction hash handling for both refund and sweep monitoring - Real-world defect scenario using actual transaction hash from logs - Edge cases including full refunds and negative amount protection The tests validate that the "insufficient funds" error has been resolved by ensuring payments with existing swept_tx_hash are not swept again.  ## Summary by CodeRabbit - New Features - Improved tracking for multi-output refund transactions, recording both refund and shop-sweep details with clearer event logs. - Bug Fixes - Prevents double-sweeping when a sweep has already occurred. - More accurate Monero confirmation checks for specific accounts/subaddresses. - Documentation - Added “Cryptocurrency RPC Access” guidance for Monero and Dogecoin. - Expanded “UUID Objects” section with usage tips and examples. - Tests - Added comprehensive tests covering multi-output refunds, sweep accounting, and XMR/DOGE edge cases. - Style - Minor formatting and string style consistency updates.

coderabbitai commented

2025-10-03 11:33:34 -04:00

(Migrated from git2.unturf.com)

Walkthrough

Adds tracking of shop-sweep data for multi-output refund transactions, updates Monero confirmation API to accept account_index, inserts related logging, expands tests for refund+sweep scenarios, updates documentation with cryptocurrency RPC access and UUID guidance, and applies cosmetic string-style changes in HTML sanitizer.

Changes

Cohort / File(s)	Summary of changes
Core: Multi-output refund & Monero confirmations `make_post_sell/lib/crypto_watcher/__init__.py`	Populate `swept_tx_hash`, `swept_amount`, and `swept_timestamp` when a refund transaction also sweeps to the shop; add logging around refund+sweep events; change `get_monero_tx_confirmations(client, tx_hash)` → `get_monero_tx_confirmations(client, tx_hash, account_index)` and update all call sites.
Tests: Multi-output transaction tracking `make_post_sell/tests/test_multi_output_transaction_tracking.py`	New test module `TestMultiOutputTransactionTracking` with tests for refund+shop-sweep behavior, shop sweep amount calculations (including DOGE specifics), prevention of double-sweep, shared TX hash handling, and a DB-tracking reproduction scenario; uses mocks for payments/invoices/shops.
Docs: RPC & UUID guidance `CLAUDE.md`	Added "Cryptocurrency RPC Access" with Monero Wallet RPC (digest auth example) and Dogecoin Core RPC (basic auth example); expanded "Common Issues and Solutions" UUID guidance to prefer `uuid_str` and note dash-free storage with example queries.
Style: HTML sanitizer `make_post_sell/lib/sanitize_html.py`	Cosmetic changes: converted single quotes to double quotes and minor whitespace/line-break adjustments; no behavioral changes.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Operator
  participant Processor as PaymentRescue/Processor
  participant DB as Database
  participant Wallet as Wallet RPC (XMR/DOGE)
  participant Log as Logger

  Operator->>Processor: trigger payment processing / refund
  Processor->>DB: load invoice/payment state
  alt refund required (over/under/expired)
    Processor->>Processor: compute refund_amount, shop_sweep_amount
    Processor->>Wallet: transfer(destinations=[customer, shop])
    Wallet-->>Processor: tx_hash, fee, status
    Processor->>DB: set refund_tx_hash, refund_amount
    alt swept_tx_hash not set
      Processor->>DB: set swept_tx_hash=tx_hash, swept_amount, swept_timestamp
    end
    Processor->>Log: log refund+sweep with shared tx_hash
  else no refund
    Processor->>Processor: normal settlement
  end
  Processor-->>Operator: result

sequenceDiagram
  autonumber
  participant Watcher
  participant XMRRPC as Monero Wallet RPC
  note over Watcher,XMRRPC: Monero confirmations require account_index
  Watcher->>Watcher: get_monero_tx_confirmations(client, tx_hash, account_index)
  Watcher->>XMRRPC: get_transfer_by_txid(txid, account_index)
  XMRRPC-->>Watcher: transfer(s) including confirmations
  Watcher-->>Watcher: return confirmations filtered by account_index

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

I balanced coins with careful sweep,
One hash to rule both refund deep —
A hop, a log, a tidy keep,
Subaddresses counted in their sleep.
Tests nibble all the edge-case peeps. 🥕🪙

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Free

📥 Commits

Reviewing files that changed from the base of the PR and between 8301e23c3f and f85cd3907c.

📒 Files selected for processing (1)

make_post_sell/tests/test_multi_output_transaction_tracking.py (1 hunks)

📝 NOTE

🎁 Summarized by CodeRabbit Free

Your organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://app.coderabbit.ai/login.

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

## Walkthrough Adds tracking of shop-sweep data for multi-output refund transactions, updates Monero confirmation API to accept account_index, inserts related logging, expands tests for refund+sweep scenarios, updates documentation with cryptocurrency RPC access and UUID guidance, and applies cosmetic string-style changes in HTML sanitizer. ## Changes | Cohort / File(s) | Summary of changes | |---|---| | **Core: Multi-output refund & Monero confirmations** `make_post_sell/lib/crypto_watcher/__init__.py` | Populate `swept_tx_hash`, `swept_amount`, and `swept_timestamp` when a refund transaction also sweeps to the shop; add logging around refund+sweep events; change `get_monero_tx_confirmations(client, tx_hash)` → `get_monero_tx_confirmations(client, tx_hash, account_index)` and update all call sites. | | **Tests: Multi-output transaction tracking** `make_post_sell/tests/test_multi_output_transaction_tracking.py` | New test module `TestMultiOutputTransactionTracking` with tests for refund+shop-sweep behavior, shop sweep amount calculations (including DOGE specifics), prevention of double-sweep, shared TX hash handling, and a DB-tracking reproduction scenario; uses mocks for payments/invoices/shops. | | **Docs: RPC & UUID guidance** `CLAUDE.md` | Added "Cryptocurrency RPC Access" with Monero Wallet RPC (digest auth example) and Dogecoin Core RPC (basic auth example); expanded "Common Issues and Solutions" UUID guidance to prefer `uuid_str` and note dash-free storage with example queries. | | **Style: HTML sanitizer** `make_post_sell/lib/sanitize_html.py` | Cosmetic changes: converted single quotes to double quotes and minor whitespace/line-break adjustments; no behavioral changes. | ## Sequence Diagram(s) ```mermaid sequenceDiagram autonumber actor Operator participant Processor as PaymentRescue/Processor participant DB as Database participant Wallet as Wallet RPC (XMR/DOGE) participant Log as Logger Operator->>Processor: trigger payment processing / refund Processor->>DB: load invoice/payment state alt refund required (over/under/expired) Processor->>Processor: compute refund_amount, shop_sweep_amount Processor->>Wallet: transfer(destinations=[customer, shop]) Wallet-->>Processor: tx_hash, fee, status Processor->>DB: set refund_tx_hash, refund_amount alt swept_tx_hash not set Processor->>DB: set swept_tx_hash=tx_hash, swept_amount, swept_timestamp end Processor->>Log: log refund+sweep with shared tx_hash else no refund Processor->>Processor: normal settlement end Processor-->>Operator: result ``` ```mermaid sequenceDiagram autonumber participant Watcher participant XMRRPC as Monero Wallet RPC note over Watcher,XMRRPC: Monero confirmations require account_index Watcher->>Watcher: get_monero_tx_confirmations(client, tx_hash, account_index) Watcher->>XMRRPC: get_transfer_by_txid(txid, account_index) XMRRPC-->>Watcher: transfer(s) including confirmations Watcher-->>Watcher: return confirmations filtered by account_index ``` ## Estimated code review effort 🎯 4 (Complex) | ⏱️ ~60 minutes ## Poem > I balanced coins with careful sweep, > One hash to rule both refund deep — > A hop, a log, a tidy keep, > Subaddresses counted in their sleep. > Tests nibble all the edge-case peeps. 🥕🪙  --- <details> <summary>📜 Recent review details</summary> **Configuration used**: CodeRabbit UI **Review profile**: CHILL **Plan**: Free <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 8301e23c3f9328b96cc20d29237c9da696b1facc and f85cd3907cdbdd6fca5db65f6358275bfd008193. </details> <details> <summary>📒 Files selected for processing (1)</summary> * `make_post_sell/tests/test_multi_output_transaction_tracking.py` (1 hunks) </details> </details>  --- > 📝 **NOTE** > <details> > <summary>🎁 Summarized by CodeRabbit Free</summary> > > Your organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting <https://app.coderabbit.ai/login>. > > </details> Comment `@coderabbitai help` to get the list of available commands and usage tips.

russellballestrini (Migrated from git2.unturf.com) scheduled this pull request to auto merge when all checks succeed 2025-10-03 11:33:56 -04:00

coderabbitai commented

2025-10-03 11:34:07 -04:00

(Migrated from git2.unturf.com)

changed the description

russellballestrini commented

2025-10-03 11:40:42 -04:00

(Migrated from git2.unturf.com)

aborted the automatic merge because source branch was updated

russellballestrini commented

2025-10-03 11:40:42 -04:00

(Migrated from git2.unturf.com)

added 1 commit

f85cd390 - Fix SQLAlchemy initialization issue in multi-output transaction tests

Compare with previous version

added 1 commit <ul><li>f85cd390 - Fix SQLAlchemy initialization issue in multi-output transaction tests</li></ul> [Compare with previous version](/engineering/make-post-sell/make_post_sell/-/merge_requests/49/diffs?diff_id=735&start_sha=8301e23c3f9328b96cc20d29237c9da696b1facc)

coderabbitai commented

2025-10-03 11:41:22 -04:00

(Migrated from git2.unturf.com)

changed the description

russellballestrini (Migrated from git2.unturf.com) scheduled this pull request to auto merge when all checks succeed 2025-10-03 11:41:30 -04:00

russellballestrini commented

2025-10-03 11:45:00 -04:00

(Migrated from git2.unturf.com)

mentioned in commit b82c6269bc

mentioned in commit b82c6269bc3a62ee0ff9ae1439cae3c58e8018d8

russellballestrini (Migrated from git2.unturf.com) merged commit b82c6269bc into master

2025-10-03 11:45:00 -04:00

Rows
Columns

Add comprehensive tests for multi-output transaction tracking defect fix #85

Summary by CodeRabbit

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Poem