GH-605: Review 3 #709

utkarshg6 · 2025-09-20T06:46:57Z

Note

Migrates payables to explicit SentTx/FailedTx with receipt-driven status (TxReceiptsMessage), drops pending-payable DAO/table, introduces detailed scan types/scheduling, and updates APIs, serialization, defaults, and tests.

Accounting/Scanning:
- Replace fingerprint-based pending-payable flow with SentTx/FailedTx records and receipt-driven status via TxReceiptsMessage and StatusReadFromReceiptCheck/TxBlock.
- Remove PendingPayableDao usage; add caches and interpreter for tx receipts; update comparisons/ordering and logging.
- Introduce DetailedScanType, Retry hints, and ScanReschedulingAfterEarlyStop naming; adjust scheduling APIs.
- Rename Tx → SentTx and fields to amount_minor/gas_price_minor; propagate across builders, SQL, and assertions.
- Enhance ValidationStatus with PreviousAttempts (custom serde/ordering) and error-kind restructuring.
Blockchain Bridge/Interface:
- Switch receipt processing to map of TxHashByTable → TxReceiptResult; add RegisterNewPendingPayables.
- Replace legacy receipt/status types; adapt agent submission error to structured form.
Database:
- Drop pending_payable table; migrations updated; config defaults now chain-aware for ScanIntervals.
CLI/Config:
- Validate payment thresholds and scan-interval relationships; compute scan-interval defaults per chain.
Tests:
- Broad updates to reflect new types, messages, scheduling, and logging; new mocks for serializer and caches.

^{Written by Cursor Bugbot for commit 02d98fb. This will update automatically on new commits. Configure here.}

* GH-598-json-hotfix: corrected * GH-598-json-hotfix: little fix * GH-598-json-hotfix: rearrangement * GH-598-json-hotfix: addressing the review * GH-598-json-hotfix: finished addressing review --------- Co-authored-by: Bert <[email protected]>

* GH-642: interim commit * GH-642: interim commit * GH-642: interim commit * GH-642: big initial messy reconstruction continuing... * GH-642: big initial messy reconstruction... just realized I may've forgotten to update with the last changes from the other card * GH-642: tests compiling...failing lots of them * GH-642: fn confirm_transactions has been reimplemented * GH-642: fn handle_failed_transactions has been reimplemented * GH-642: fixed mainly internal, but smaller functions in the pending payable scanner; various From and Display implementations and these sorts * GH-642: progressed quite greatelly; fixed many tests; took action against the mark pending payable rowid fn * GH-642: another bunch fixed...down to 24 * GH-642: another bunch fixed...down to 10 * GH-642: the base of this card is done * GH-642: lots of fixes in names * GH-642: filling cache with failed txs to recheck at startup * GH-642: rpc failers during receipt checks can be handled now * GH-642: interim commit * GH-642: first I need to finish the impl of the db system of tx statuses...opened in its own PR and then will start from here on * GH-642: pending payable scanner machinery has been given the true skeleton * GH-642: before creating a new whole folder for scanner utils * GH-642: interim commit * GH-642: preparing tests before writing the guts of the core fns * GH-642: integration of the caches...100% at start_scan, 90% finish_scan * GH-642: another big portion of work in interpreting the receipts * GH-642: finishing tests for the receipt interpretation but I should rearrange the code a bit - maybe to add a separative class * GH-642: mod structure changed, new file for TxReceiptInterpreter * GH-642: fixed two unreliable tests * GH-642: interim commit * GH-642: worked away on the implementation of handling failed txs * GH-642: more todos!() gone * GH-642: processing failures is done; next tx confiramtions * GH-642: tx reclaim implemented * GH-642: finished the brain functions in PPS * GH-642: ValidationStatus extension - huge chunk of work; still some failing tests remain * GH-642: interim commit (some of the Validation error stuff will have to be fixed) * GH-683: savepoint * GH-683: interim commit * GH-683: mostly done * GH-683: renamed error * GH-683: additional fix to renaming * GH-683: finished * GH-642: finished * GH-683: fixed for a review * GH-683: fixed screwed string replacement * GH-683: finished fixing it * GH-683: another fix...BlockchainError * GH-642: added unreachable! * GH-642: before bigger issue addressing * GH-642: got rid of the BlockchainFailure::Unrecognized layer * GH-642: savepoint * GH-642: hashmap for receipt status result deployed * GH-642: finally solid... as much as under this card, tests fixed * GH-598-json-hotfix: interim commit * GH-642: dragging the failing tests down to bare minimum * GH-642: interim commit * GH-642: before fixing the todo!() left over * GH-642: finished * GH-642: cosmetics * GH-642: grrr - cosmetics - forgot e * GH-642: review 2 addressed * GH-642: added the cache clean-up on getting a scan error --------- Co-authored-by: Bert <[email protected]>

* GH-689: finished * GH-689: fixes after auto-review --------- Co-authored-by: Bert <[email protected]>

* GH-606-v2: db and help msgs + the interface document * GH-606-v2: constraint/raletion between scan intervals established * GH-606: final clean-up * GH-606-v2: after review * GH-606-v2: formatting --------- Co-authored-by: Bert <[email protected]>

…ry_tx_templates

…_priced_retry_tx_templates

node/src/accountant/scanners/mod.rs

bertllll · 2025-09-21T10:39:37Z

node/src/accountant/scanners/payable_scanner/tx_templates/priced/new.rs

    ) -> String {
        format!(
-            "The computed gas price {} wei is above the ceil value of {} wei set by the Node.\n\
+            "The computed gas price {} wei is above the ceil value of {} wei computed by this Node.\n\


The word "ceil" is a nonsense. And I don't think this kind of shortcut is widely used. 😬

This is a commonly used word in programming terminology. See:

Example 1 - JavaScript

Example 2 - C

Example 3 - Python

Example 4 - Rust

I suggest you get used to it, it's short and it conveys what we mean.

But only among programmers and I wouldn't question it as a function name. This message is not necessarily meant for programmers. "Ceil" as a word doesn't exist and it probably is not used as a shortcut either. You should be looking into the linguistic dictionary, not the programming language documentation.

Here you go - https://www.dictionary.com/browse/ceil

node/src/accountant/scanners/payable_scanner/tx_templates/test_utils.rs

node/src/accountant/test_utils.rs

bertllll · 2025-09-21T10:59:22Z

node/src/blockchain/blockchain_interface/blockchain_interface_web3/utils.rs

 }

+fn calculate_payments_column_width(signable_tx_templates: &SignableTxTemplates) -> usize {
+    let label_length = "[payment wei]".len();


This can easily be a constant const LABEL_LENGTH: usize = 13; // "[payment wei]"
You can leave the constant definition inside this fn. It's convenient (not polluting the outer scope).

Don't you think it provides better readability? I personally find it more ergonomic.

Nope. I think it is silly to write it like this because there is no guarantee it matches the true string there outside this fn. Using a dynamical determination of this value seems like an overkill because you can easily count the letters up.

The current approach provides clarity by showing that the length is derived directly from the label itself. This can make the code easier to understand for someone reading it, as they can see the relationship between the label and its length.

bertllll

Well done. Just a few last things please.

…r PartialOrd and Ord

cursor · 2025-09-29T12:18:02Z

node/src/accountant/db_access_objects/failed_payable_dao.rs

+    pub amount_minor: u128,
    pub timestamp: i64,
-    pub gas_price_wei: u128,
+    pub gas_price_minor: u128,


Bug: Transaction Structs Change Ordering Behavior

The FailedTx and SentTx structs now derive Ord instead of using custom implementations. This changes their ordering from a custom descending logic (timestamp, nonce, amount) to the default ascending order based on field declaration. This behavioral shift may impact components relying on the previous ordering, such as BTreeSets or transaction processing logic.

Additional Locations (1)

node/src/accountant/db_access_objects/sent_payable_dao.rs#L30-L31

cursor · 2025-09-29T12:18:02Z

node/src/accountant/scanners/pending_payable_scanner/tx_receipt_interpreter.rs

+                            failed_tx.hash
+                        )
+                    })
+                    .hash;


Bug: Transaction Handling Crashes on Unexpected States

In handle_still_pending_tx, the code panics if a failed transaction is found pending for a reason other than PendingTooLong (via unreachable!()), or if a replacement transaction isn't found for a PendingTooLong failure. These strict assumptions about transaction states and database consistency can lead to application crashes.

bertllll and others added 23 commits September 9, 2025 10:57

GH-598-json-hotfix (#699)

24e0a8a

* GH-598-json-hotfix: corrected * GH-598-json-hotfix: little fix * GH-598-json-hotfix: rearrangement * GH-598-json-hotfix: addressing the review * GH-598-json-hotfix: finished addressing review --------- Co-authored-by: Bert <[email protected]>

GH-689: Amend scanner scheduling: Handling ScanError msg (#691)

6e020c7

* GH-689: finished * GH-689: fixes after auto-review --------- Co-authored-by: Bert <[email protected]>

GH-605: add TODO for the test

24396b1

GH-605: more and more changes

d2fd9cd

GH-605: bit more refactoring

52411c9

GH-605: bit more changes

398bdd9

GH-605: changing to & works

e044dc4

GH-605: further changes

8227dc8

GH-605: more TODOs

ff6e400

GH-605: derive Copy for PayableScanType

dc863ef

GH-605: add scan_type function

9fd9ee7

GH-605: comment out unused code in test

9ba398e

GH-605: refactor the code a bit

80711d4

GH-605: refactor signable_tx_templates_can_be_created_from_priced_ret…

4ac6418

…ry_tx_templates

GH-605: the helper functions have more realistic args name

83d6155

GH-605: more refactoring of signable_tx_templates_can_be_created_from…

141d10a

…_priced_retry_tx_templates

GH-605: more refactored changes

4f91045

GH-605: further refactoring changes

896f7d0

GH-605: handle_batch_results has been renamed

18e0a54

GH-605: few more changes

cf0938a

GH-605: few more changes

9308d37