Conversation
* GH-601: introduce sent_payable table * GH-601: test that table with certain columns were created during migration * GH-601: resuse code to test fields of table * GH-601: migrate utils code * GH-601: trigger actions * GH-601: fix test constants_have_correct_values * GH-601: review changes * GH-601: move constant to database/test_utils.rs
…manily its DAO) (#617) * GH-608: change the trait object to support SentPayables * Revert "GH-608: change the trait object to support SentPayables" This reverts commit df415c0. * GH-608: introduce sent payable dao * GH-608: add boilerplate code * GH-608: implement insert_new_records * GH-608: add test cases for the panic and error clause for the insert method * GH-608: introduce builder pattern for the Tx * GH-608: implement get_tx_identifiers * GH-608: implement retrieve_pending_txs() * GH-608: add more assertions for the retrieve_pending_txs() and implement Display for TxStatus * GH-608: rename time_t in fn names to unix_timestamp * GH-608: refactor sent_payable_dao.rs * GH-608: add stronger tests for insert_new_records() * GH-608: retrieve txs conditionally * GH-608: change SQL to uppercase wherever necessary * GH-608: add RetrieveCondtion::ToRetry * GH-608: add constant RETRY_THRESHOLD_SECS * GH-608: implement display and from_str() for TxStatus * GH-608: implement retrieve_condition_display_works * GH-608: update the query for txs_to_retry and reorder trait functions * GH-608: change SQL query for txs to retry * GH-608: remove RETRY_THRESHOLD_SECS * GH-608: remove retried column from the sent_payable table * GH-608: add ability to retrieve tx by hash * GH-608: add the ability to update statuses * GH-608: add types for TxHash and RowID * GH-608: add test for deleting records * GH-608: add fn for deleting records * GH-608: add more TODOs * GH-608: error testing * GH-608: write tests for error handling while deleting records * GH-608: add more TODOs * GH-608: return SqlExecutionFailed in insert, change_status and delete operations * GH-608: add better error handling for delete records * GH-608: change the signature of delete_records() to accept Hashset * GH-608: remove unused SentPayableDaoError * GH-608: use the variant EmptyInput * GH-608: add more validations for insert_new_records * GH-608: test all errors * GH-608: remove unnecessary TODOs * GH-608: perform some cleanup * GH-608: eliminate clippy warnings * GH-608: review 1 changes * GH-608: remove clippy warnings * GH-608: review 2 changes
* GH-602: big bunch of changes * GH-602: interim commit * GH-602: four scans layout installed * GH-602: mostly done but still some crumbs of work left * GH-602: interim commit * GH-602: Recorder enriched by counter messages * GH-602: removed obfuscated and made right the formerly developed StopConditions * GH-602: removed obfuscated and made right the formerly developed StopConditions * GH-602: main part probably done * GH-602: more todos knocked off in accountant/mod.rs * GH-602: deep dive into schedulers and the awarness of the schedule state * GH-602: save point before going into fully private scanner * GH-602: another save point, this time before turning the scanner into fully private object * GH-602: finally fully scanners as flexible parametrized generic objects * GH-602: todos placed (many) as understood as the whole design * GH-602: more cases covered...knocking off todos!() continues * GH-602: made a markup for upcoming steps * GH-602: made a markup for upcoming steps * GH-602: Manual scanners prevented by the automation. Tested * GH-602: More tests written; mainly constraining the manual scanning * GH-602: before trying to implement the ScanScheduleHintErrorResolver * GH-602: tests for pending payable scanner and its start scan error resolver are compiling * GH-602: all tests in scan_schedulers passing now * GH-602: all tests in scan_schedulers passing now * GH-602: slowly fixing test in accountant/mod.rs * GH-602: fixed another couple of failing tests in accountant/mod.rs and scanners/mod.rs * GH-602: adding another little pile * GH-602: closing the gap... probably only one test to be written yet * GH-602: one more test written...ext tr. pen payable * GH-602: eliminated last todos in scanners/mod.rs * GH-602: first rough clean-up * GH-602: some comments fixed * GH-602: first piece of the review finished * GH-602: addressing review continues... * GH-602: addressing review continues on and on... * GH-602: more things for the review... * GH-602: last 6 comments to address * GH-602: finished the big two tests covering the very start of the automatic process with pp scanner playing the first violin * GH-602: huge tests refactored * GH-602: small functionalities made right and huge tests refactored * GH-602: prepared some fixes for early rescheduling * GH-602: moved the scanner test utils * GH-602: counter message summary comment added * GH-602: probably finished the minimm of what can be submitted seriously * GH-602: removed commented out impl * GH-602: vast majority of the review is covered * GH-602: refactoring in the biggeste tests - a la utkarsh * GH-602: review two ends * GH-602: improved comments --------- Co-authored-by: Bert <Bert@Bert.com>
* GH-631: store gas price wei in two columns * GH-631: more updates to the sent payable table * GH-631: eliminate all errors * GH-631: accept null values in db * GH-631: all tests passing * GH-631: use TransactionBlock everywhere * GH-631: enforce that block details are complete * GH-631: fix test constants_have_correct_values * GH-631: introduce failed payable table * GH-631: add db migration for failed payable table * GH-631: mid review changes * GH-631: wip: introduce failed_payable_dao.rs * GH-631: introduce FailedTxBuilder and updated trait for FailedPayableDao * GH-631: insert_new_txs() works * GH-631: txs can be retrieved from FailedPayables * GH-631: implement all trait fns for FailedPayables * GH-631: add a test for string conversion for FailureReason * GH-631: make improvements to failed_payable_dao * GH-631: remove usages of H256 from the tests of DAOs * GH-631: improve imports * GH-631: introduce a utility fn make_block_hash() * GH-631: add review changes for node/src/database/db_initializer.rs * GH-631: some more review changes * GH-631: some more review changes * GH-631: change checked to a boolean * GH-631: keep pushing more review changes * GH-631: display more info in the error statements * GH-631: and even more changes * GH-631: change checked to rechecked * GH-631: remove blank line * GH-631: minor self-review changes * GH-631: add another review changes * GH-631: add more review changes * GH-631: introduce the replace_record() in SentPayableDao * v0.9.0: version bump (#657) * GH-631: review 4 changes * GH-631: review 5 changes * GH-631: review 6 changes * GH-631: import the Itertools and make join work without collect * GH-631: eliminate clippy warnings; tests will still fail due to todo!()
* GH-638: ploughed all over, but the main structure has been planted; now let's get rid of 50 c. errors * GH-638: interim commit; nice progress though * GH-638: lots done; but the computations will be hrder to test * GH-638: little rearrangement in the project tree before I bring in more files * GH-638: transmission log repair * GH-638: transmission log repair and some other tests * GH-638: tests for gas price ceiling confrontations written * GH-638: single test fixed * GH-638: tests full - all and fixed * GH-638: lint and formatting * GH-638: self-review * mend * GH-638: added warning * GH-638: another comment res * GH-638: ref tests with a lot of created accounts * GH-638: fixed chain records with more constants * GH-638: review two addressed --------- Co-authored-by: Bert <Bert@Bert.com>
* GH-605: add the steps to solve this card * GH-605: change the recheck to status * GH-605: all tests pass in failed_payable_dao.rs * GH-605: few more changes * GH-605: add instructions inside test retry_payable_scanner_can_initiate_a_scan * GH-605: introduce mocks for FailedPayableDAO * GH-665: add another variant for the reason: General * GH-665: add the string conversion for General * GH-665: review changes
* GH-672: add more errors * GH-672: improve errors; compiling * GH-672: introduce the conversion of FailureReason * GH-672: all tests are passing :) * GH-672: From conversion is properly tested * GH-672: final touches * GH-672: return String as Error instead of an error from serde * GH-672: better error classification
Co-authored-by: Bert <Bert@Bert.com>
* 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-683: fixed for a review * GH-683: fixed screwed string replacement * GH-683: finished fixing it * GH-683: another fix...BlockchainError --------- Co-authored-by: Bert <Bert@Bert.com>
* 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 <Bert@Bert.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
* GH-827: add boilerplate code * GH-827: introduce new logic * GH-827: some renamings for clarity * GH-827: fix test estimate_transaction_fee_total_works_for_retry_txs * GH-827: refactor test estimate_transaction_fee_total_works_for_retry_txs * GH-827: fix test returns_correct_priced_qualified_payables_for_retry_payable_scan * GH-827: fix test blockchain_interface_web3_can_introduce_blockchain_agent_in_the_retry_payables_mode * GH-827: fix 2 more tests * GH-827: refactor retry_payables_gas_price_ceiling_test_of_border_value_if_the_previous_attempt_being_bigger * GH-827: review changes
| f, | ||
| "WHERE receiver_address IN ({})", | ||
| join_with_commas(addresses, |address| format!("'{:?}'", address)) | ||
| ) |
There was a problem hiding this comment.
Empty collections cause invalid SQL in retrieve conditions
High Severity
When FailureRetrieveCondition::ByTxHash or ByReceiverAddresses contains an empty collection, the Display implementation generates invalid SQL like WHERE tx_hash IN (), which causes a runtime SQL error. The same issue affects RetrieveCondition::ByHash and ByNonce in sent_payable_dao.rs. This occurs in production code at payable_scanner/mod.rs:283 where retrieve_failed_txs_by_receiver_addresses is called with potentially empty sent_txs, generating an empty receiver address set.
Additional Locations (1)
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is kicking off a free cloud agent to fix this issue. This run is complimentary, but you can enable autofix for all future PRs in the Cursor dashboard.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A brief explanation of how it works, plus some instructions for testing in the attached file:
Payment mechanism testing guidlines.txt
Note
High Risk
High risk because it changes payment bookkeeping/storage (new DB tables + schema bump) and transaction confirmation logic, which can affect real fund flows and migrations. It also introduces new DAO paths with
todo!()/SQL-index changes that could cause runtime panics or incorrect persistence if exercised.Overview
Payments bookkeeping revamp: adds
sent_payableand newfailed_payabletransaction persistence (newTransactiontrait + DAOs, conversion helpers) and updates payable confirmation to operate on sent-tx records rather than the prior pending-payable fingerprint/rowid approach.DB/config changes: bumps
CURRENT_SCHEMA_VERSIONto12, extends per-chain metadata (gas_price_safe_ceiling_minor,default_pending_payable_interval_sec) with new constants, and adds aSimpleClockabstraction + new collection helper macros (btreemap!,hashset!,btreeset!) while removingtype_obfuscation.Surface/test updates: refreshes scan-interval documentation/help text (including revised
pendingPayableSecsemantics), reorders scan interval display, speeds up a mock server sleep loop, and updates multinode integration tests to use the new payable retrieval and pending-tx checks/log strings.Written by Cursor Bugbot for commit 8bd5a40. This will update automatically on new commits. Configure here.