Remove hard 5000sats min send limit#810
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe PR replaces fixed minimum-send values with shared low-send warning and conservative dust-limit constants, adds warning acknowledgement state and alert variants, updates send-flow validation and finalization, and remaps dust-limit transaction errors. ChangesSend-flow warning and dust-limit alignment
Sequence Diagram(s)sequenceDiagram
participant SendFlowSetAmountScreen
participant SendFlowPresenter
participant SendFlowManager
SendFlowSetAmountScreen->>SendFlowPresenter: alertButtons(alert)
SendFlowPresenter->>SendFlowManager: dispatch(.AcknowledgeWarningAndFinalize(kind))
SendFlowManager->>SendFlowManager: finalize_and_go_to_next_screen()
SendFlowManager->>SendFlowPresenter: clearAlert()
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
praveenperera
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 027a2ea8f0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if me.selected_fee_rate().and_then(|fee| fee.total_fee).is_none() { | ||
| return me | ||
| .send_alert_async(SendFlowError::UnableToGetFeeDetails( | ||
| "selected fee total unavailable".to_string(), | ||
| )) |
There was a problem hiding this comment.
When the selected fee option has no total_fee (common on a fresh flow before totals are calculated), a below-dust amount makes get_or_update_fee_rate_options() fail to populate totals because BDK rejects the candidate output. This branch then surfaces UnableToGetFeeDetails("selected fee total unavailable") and returns before confirm_txn can map BDK's OutputBelowDustLimit into the new friendly dust error, so users sending e.g. 1 sat see a misleading fee-details failure instead of the intended dust-limit guidance.
Useful? React with 👍 / 👎.
Greptile SummaryThis PR moves small on-chain sends from hard limits to warning and dust-validation flows. The main changes are:
Confidence Score: 4/5This is close, but the iOS coin-control gate should be fixed before merging.
ios/Cove/Flows/CoinControlFlow/UtxoListScreen.swift Important Files Changed
Reviews (5): Last reviewed commit: "Recheck send snapshot before PSBT result" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@android/app/src/main/java/org/bitcoinppl/cove/flows/SendFlow/CoinControlCustomAmountSheet.kt`:
- Around line 76-81: The max send calculation in CoinControlCustomAmountSheet
can collapse the slider range to a single point when maxSendMinusFees() returns
a value at or below minSend. Update the maxSend logic so it is clamped to a real
upper bound like conservativeDustLimitSatsU + 1_000uL (matching the iOS
behavior) rather than only coerceAtLeast(minSend), and keep the Slider
valueRange in sync with that safer bound.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 255cf688-e98a-4434-be4a-f99966a14472
⛔ Files ignored due to path filters (2)
android/app/src/main/java/org/bitcoinppl/cove_core/cove.ktis excluded by!android/app/src/main/java/org/bitcoinppl/cove_core/**ios/CoveCore/Sources/CoveCore/generated/cove.swiftis excluded by!**/generated/**,!ios/CoveCore/Sources/CoveCore/generated/**
📒 Files selected for processing (25)
android/app/src/main/java/org/bitcoinppl/cove/flows/SendFlow/CoinControlCustomAmountSheet.ktandroid/app/src/main/java/org/bitcoinppl/cove/flows/SendFlow/SendFlowContainer.ktandroid/app/src/main/java/org/bitcoinppl/cove/flows/SendFlow/SendFlowManager.ktandroid/app/src/main/java/org/bitcoinppl/cove/flows/SendFlow/SendFlowPresenter.ktios/Cove/Constants.swiftios/Cove/Flows/CoinControlFlow/UtxoListScreen.swiftios/Cove/Flows/SendFlow/Common/SendFlowUtxoCustomAmountSheetView.swiftios/Cove/Flows/SendFlow/SendFlowCoinControlSetAmountScreen.swiftios/Cove/Flows/SendFlow/SendFlowManager.swiftios/Cove/Flows/SendFlow/SendFlowPresenter.swiftios/Cove/Flows/SendFlow/SendFlowSetAmountScreen.swiftrust/crates/cove-common/src/consts.rsrust/src/manager/send_flow_manager.rsrust/src/manager/send_flow_manager/address_input.rsrust/src/manager/send_flow_manager/alert_state.rsrust/src/manager/send_flow_manager/amount_input.rsrust/src/manager/send_flow_manager/coin_control.rsrust/src/manager/send_flow_manager/error.rsrust/src/manager/send_flow_manager/fee_selection.rsrust/src/manager/send_flow_manager/finalize.rsrust/src/manager/send_flow_manager/state.rsrust/src/manager/wallet_manager.rsrust/src/manager/wallet_manager/actor.rsrust/src/manager/wallet_manager/actor/transactions.rsrust/src/wallet/ffi.rs
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
Addressed the remaining review findings through Changes:
Verification passed:
Final review passes:
|
praveenperera
force-pushed
the
remove-send-limit
branch
3 times, most recently
from
1c8f9bb to
9d4a15b
Compare
Replace the 5000-sat hard floor with Rust-owned finalize warnings, keep BDK dust rejection as the hard policy, and update generated iOS and Android bindings plus platform alert handling.
Surface BDK dust failures when fee totals cannot be refreshed. Update locked-output tests for the new insufficient-funds mapping. Delay chained iOS alerts during dismissal.
Handle exact-dust max-send slider bounds on both mobile platforms and delay Android alert re-presentation while an existing alert is dismissing.
Let coin-control amount entry keep values below the conservative dust fallback so recipient-specific dust validation can decide whether the send is valid.
Let coin-control amount updates continue when the max-send snap threshold cannot be computed, so small selected coin sets still preserve the user's entered amount for later dust validation.
Keep coin-control entries above the fee-adjusted selected-coin max in max mode when the small-UTXO snap threshold is unavailable.
Abort the async fee fallback when the amount, address, mode, or fee choice changed while fee totals were loading. Also block coin-control sends when the selected fee exhausts the selected coins.
praveenperera
force-pushed
the
remove-send-limit
branch
from
9d4a15b to
5847249
Compare
| @@ -254,7 +254,7 @@ struct UtxoListScreen: View { | |||
| .frame(maxWidth: .infinity) | |||
| .padding(.horizontal) | |||
| .padding(.bottom, 4) | |||
| .disabled(manager.totalSelectedSats < minSendSats) | |||
| .disabled(manager.totalSelectedSats < conservativeDustLimitSats) | |||
There was a problem hiding this comment.
conservativeDustLimitSats. A user with a 300-sat selection cannot enter the send flow, even though that amount can be valid for lower-dust recipient scripts once the address is entered and BDK applies the script-specific dust check. This leaves a hard 546-sat platform gate in front of the new finalize-time validation path.
Updated `UtxoListScreen` to use a new `canContinue` computed property for the Continue button state and styling. The button is now enabled when there is at least one selected UTXO and the selected total is greater than zero, instead of being blocked by the conservative dust-limit threshold.
1 participant
Summary
Replaces Cove's 5000-sat hard send floor with finalize-time warnings for small
on-chain payments and high fee percentages.
Rust now owns the warning state, acknowledgement flow, reset behavior, and hard
validation ordering. BDK transaction construction remains authoritative for
recipient dust rejection, which is mapped to a friendly cross-platform dust
error.
iOS and Android render the new warning alerts, dispatch
Send Anywayback toRust, use the conservative dust fallback for coin-control floors, and consume
regenerated UniFFI bindings.
Verification
cargo test -p cove --lib send_flow_managercargo test -p cove --lib create_tx_cargo test -p cove --lib actor_manual_max_send_uses_recipient_exact_dust_floorjust fmtjust clippyjust build-iosjust build-androidjust compile-iosjust compile-androidgit diff --checkSummary by CodeRabbit