sweep: isolate bad inputs on mempool rejection#10842
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a diagnostic mechanism for the sweeper to identify and isolate problematic inputs when a transaction batch is rejected by the mempool. By performing no-broadcast probes on subsets of inputs, the system can now pinpoint specific inputs causing failures rather than failing the entire batch. This improves the robustness of the sweeping process by ensuring that only truly invalid inputs are treated as fatal, while valid inputs can continue to be processed and retried. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a mechanism to isolate and diagnose bad inputs in a rejected sweep batch using no-broadcast mempool acceptance probes. When a multi-input batch is rejected with an unattributed non-fee mempool or script error, the TxPublisher performs a binary search probe of the input subsets to identify the specific inputs causing the failure. The UtxoSweeper then marks only those identified bad inputs as fatal, allowing the remaining valid inputs in the batch to be retried. A review comment suggests adding a defensive nil check for r.feeFunction in shouldDiagnoseBadInputs to prevent a potential nil pointer dereference panic.
🔴 PR Severity: CRITICAL
🔴 Critical (2 files)
🟢 Low (2 files)
AnalysisThis PR modifies The bulk of changes (265 lines) are in Severity bump check: 4 non-test files (threshold: >20), 417 non-test lines (threshold: >500), single critical package — no bump applied. To override, add a |
87d1e3e to
75b9fe1
Compare
75b9fe1 to
4ef522b
Compare
|
LGTM ⚡ ( |
4ef522b to
b1443e9
Compare
|
LGTM 🦦 |
|
LGTM ✨ ( |
a5f51c3 to
24a2759
Compare
|
No issues found by |
|
No issues found by openai/gpt-5.5 ✅ |
24a2759 to
7ca790c
Compare
7ca790c to
7991909
Compare
Mark errors returned from mempool acceptance checks so later logic can distinguish rejected transactions from construction, signing, or fee-selection failures.
Add the eligibility checks for no-broadcast bad-input probes and cover the cases that must keep the existing whole-set failure path.
Add no-broadcast probe transactions for input subsets, preserving missing-input handling and returning concrete mempool policy errors to the caller.
Identify the mempool rejection classes that can be attributed to a single input script or witness, leaving broader policy failures for the caller to handle without fatal attribution.
Add the final-shape bad-input search helper that recursively probes subsets without broadcasting, and cover both singleton attribution and non-attributable policy failures.
Wire bad-input diagnosis into the non-fee mempool rejection path so a singleton script failure becomes a retryable failed batch annotated with the diagnosed input.
Cover non-attributable diagnosis outcomes, probe construction and missing-input errors, and initial-broadcast paths that must preserve the existing whole-set fatal behavior.
Handle diagnosed bad-input results in the sweeper by marking only the isolated input fatal and returning the rest of the set to the normal retry flow.
7991909 to
e111613
Compare
|
|
||
| badInput, probeErr := t.findBadInput(r) | ||
| if errors.Is(probeErr, ErrInputMissing) { | ||
| return t.handleMissingInputs(r) |
There was a problem hiding this comment.
This call is introduced in 8df710517, but handleMissingInputs is only made nil-r.tx safe in the later 0c86e21e5 test commit. That means checking out or bisecting the implementation commit can panic on the initial-broadcast probe-missing-input path before a tx exists, and the production fix is hidden in a sweep/test: commit. Please fold the nil guard into the commit that adds this call, or split the test commit so each commit is self-contained. (openai/gpt-5.5)
| // input-standardness failures narrow the search left. | ||
| // Missing inputs keep their dedicated handling. Policy/backend | ||
| // errors stop the search without marking any input fatal. | ||
| err := t.probeInputSet(r, left) |
There was a problem hiding this comment.
Probing only left can abort diagnosis before the actual bad input is inspected. If the set needed wallet inputs, or the left half contains only a required-output HTLC, probeInputSet returns a construction error such as ErrNotEnoughInputs; this default path returns that error and handleBadInputs later emits TxFailed with no BadInput, so the original cluster can be retried unchanged forever even when the bad input is in the right half. Please keep subset probes constructible, or continue with the other half/a singleton scan when a subset construction error is hit, so one invalid input does not pin the rest of the batch. (openai/gpt-5.5)
|
No issues found by |
Summary
Fixes #10840.
Tests