Skip to content

Commit 337b47b

Browse files
committed
fix(fees): validate against live minimumFee + resolve audit findings
Validate custom fees against the mempool minimumFee instead of a hardcoded 0.1 (i5hi's 'custom fee validation > minimum'). The precise endpoint's minimumFee was parsed then discarded; FeeOptions now carries a minRelay rate = max(minimumFee, 0.1), the mapper floors every tier at it, and the custom-fee field, RBF gate, and send/swap commit gates reject anything below it via aboveMinRelay(floorSatPerKwu:). Under congestion the app no longer accepts a fee the network won't relay; never drops below the 0.1 safety floor. fix(swap): the two Bitcoin fee-preview builders hardcoded drain:false while the commit path uses drain:isMaxSelected, so a max-send could cache and broadcast a non-draining PSBT (wrong amount, residual left in wallet). Pass the real flag, mirroring SendCubit. fix(fees): a malformed-but-200 precise response (missing/non-numeric field) threw uncaught instead of falling back — parsing now happens inside _getFees so it falls back to the recommended endpoint. fix(fees): clearing the custom-fee field (abs/rel toggle flip or emptied input) now disarms the parent, so a stale pre-clear value can't commit on dismissal. fix(send): preserve isToSelf on the cached-PSBT commit instead of hardcoding false (was flipping the to-self badge and mis-gating payjoin on self-sends). chore(fees): localize the preset-row unit labels and the Estimated-delivery prefix; correct the mapper doc on minimumFee. Adds regression tests for all of the above.
1 parent 0d24196 commit 337b47b

15 files changed

Lines changed: 268 additions & 56 deletions

lib/core/fees/data/fees_datasource.dart

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -60,31 +60,38 @@ class FeesDatasource {
6060

6161
final http = _dioBuilder(baseUrl);
6262

63-
final json =
63+
final fees =
6464
await _getFees(http, ApiServiceConstants.mempoolPreciseFeesPath) ??
6565
await _getFees(http, ApiServiceConstants.mempoolRecommendedFeesPath);
66-
if (json == null) {
66+
if (fees == null) {
6767
throw MempoolFeesException(
6868
'No mempool fee endpoint available at $baseUrl',
6969
);
7070
}
7171

72-
return MempoolFeesModel.fromJson(json);
72+
return fees;
7373
}
7474

75-
/// GETs a fee endpoint. Returns the JSON body on a 200 with a JSON object,
76-
/// or `null` on any failure (non-200, network/Dio error, or a non-object
77-
/// body) so the caller can fall back to another path.
78-
Future<Map<String, dynamic>?> _getFees(Dio http, String path) async {
75+
/// GETs a fee endpoint and parses it. Returns the model on a 200 with a
76+
/// well-formed body, or `null` on any failure — non-200, network/Dio
77+
/// error, non-object body, or a 200 whose body is missing or has a
78+
/// non-numeric fee field — so the caller can fall back to the next path.
79+
/// Parsing happens here (not at the call site) so a malformed-but-200
80+
/// precise response falls back to recommended instead of throwing.
81+
Future<MempoolFeesModel?> _getFees(Dio http, String path) async {
7982
try {
8083
final resp = await http.get<dynamic>(path);
8184
final data = resp.data;
8285
if (resp.statusCode == 200 && data is Map<String, dynamic>) {
83-
return data;
86+
return MempoolFeesModel.fromJson(data);
8487
}
8588
return null;
8689
} on DioException {
8790
return null;
91+
} catch (_) {
92+
// A 200 with a malformed/partial body — `fromJson` throws on a missing
93+
// or non-numeric fee field. Fall back rather than failing the fetch.
94+
return null;
8895
}
8996
}
9097
}

lib/core/fees/data/fees_repository_impl.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ class FeesRepositoryImpl implements FeesRepository {
2626
fastest: minRelay,
2727
economic: minRelay,
2828
slow: minRelay,
29+
minRelay: minRelay,
2930
);
3031
}
3132
}

lib/core/fees/data/mappers/mempool_fees_mapper.dart

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -10,28 +10,34 @@ import 'package:bb_mobile/core/fees/domain/fees_entity.dart';
1010
/// - **Economic** ← `hourFee` (~1-hour target).
1111
/// - **Slow** ← `economyFee`.
1212
///
13-
/// Every tier is floored at the network minrelayfee so no preset can drop
14-
/// below what the network will relay. Because mempool returns the fields in
15-
/// non-increasing order (`fastestFee ≥ hourFee ≥ economyFee`) and `max` is
16-
/// monotonic, flooring all three preserves the Fastest ≥ Economic ≥ Slow
17-
/// ordering even in the pathological case where a server reports rates below
18-
/// the floor (which would otherwise invert Slow above Economic).
13+
/// The relay floor is `max(minimumFee, 0.1 sat/vByte)`: the live mempool
14+
/// `minimumFee` (the rate below which that node won't relay — typically 0.1
15+
/// at quiet blocks, higher under congestion), clamped up to the static 0.1
16+
/// safety floor. Every tier is floored at it, and it is exposed as
17+
/// [FeeOptions.minRelay] so the validation gates reject anything below the
18+
/// network's current minimum rather than a hardcoded constant. Because
19+
/// mempool returns the fields in non-increasing order (`fastestFee ≥ hourFee
20+
/// ≥ economyFee ≥ minimumFee`) and `max` is monotonic, flooring all three
21+
/// preserves the Fastest ≥ Economic ≥ Slow ordering.
1922
///
20-
/// `halfHourFee` and `minimumFee` are intentionally unused — the app exposes
21-
/// exactly three tiers, and Slow is floor-protected rather than tracking
22-
/// mempool's `minimumFee` (which collapses to ~1 sat/vByte at quiet blocks
23-
/// and would make Slow indistinguishable from a real economy rate).
23+
/// `halfHourFee` is intentionally unused — the app exposes exactly three
24+
/// tiers. `minimumFee` feeds only the relay floor (above), never a tier, so
25+
/// Slow stays a real economy rate instead of collapsing onto the floor.
2426
class MempoolFeesMapper {
2527
const MempoolFeesMapper._();
2628

2729
static FeeOptions toFeeOptions(MempoolFeesModel model) {
28-
RelativeFee tier(double satPerVbyte) => NetworkFee.relativeFromSatPerVbyte(
29-
max(satPerVbyte, NetworkFeeRelayPolicy.minRelaySatPerVbyte),
30+
final floorSatPerVbyte = max(
31+
model.minimumFee,
32+
NetworkFeeRelayPolicy.minRelaySatPerVbyte,
3033
);
34+
RelativeFee tier(double satPerVbyte) =>
35+
NetworkFee.relativeFromSatPerVbyte(max(satPerVbyte, floorSatPerVbyte));
3136
return FeeOptions(
3237
fastest: tier(model.fastestFee),
3338
economic: tier(model.hourFee),
3439
slow: tier(model.economyFee),
40+
minRelay: NetworkFee.relativeFromSatPerVbyte(floorSatPerVbyte),
3541
);
3642
}
3743
}

lib/core/fees/domain/fees_entity.dart

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -105,15 +105,25 @@ extension NetworkFeeRelayPolicy on NetworkFee {
105105
static const double minRelaySatPerVbyte = 0.1;
106106
static const int minRelaySatPerKwu = 25;
107107

108-
/// True when this fee is at or above the network minrelayfee. Absolute
109-
/// fees need a [txSize] to express as a rate; [txSize] ≤ 0 → false (the
110-
/// caller is in a transient pre-build state and shouldn't be allowed to
111-
/// commit yet).
112-
bool aboveMinRelay({int? txSize}) => switch (this) {
113-
RelativeFee(:final satPerKwu) => satPerKwu >= minRelaySatPerKwu,
114-
AbsoluteFee(:final sats) =>
115-
txSize != null && txSize > 0 && (sats / txSize) >= minRelaySatPerVbyte,
116-
};
108+
/// True when this fee is at or above the relay floor. The floor defaults
109+
/// to the static [minRelaySatPerKwu] (0.1 sat/vByte) but callers SHOULD
110+
/// pass [floorSatPerKwu] from the live mempool `minimumFee`
111+
/// ([FeeOptions.minRelay]) so the gate tracks the network's current
112+
/// minimum during congestion — never below the static 0.1 safety floor,
113+
/// since [MempoolFeesMapper] takes `max(minimumFee, 0.1)`.
114+
///
115+
/// Absolute fees need a [txSize] to express as a rate; [txSize] ≤ 0 →
116+
/// false (the caller is in a transient pre-build state and shouldn't be
117+
/// allowed to commit yet). The absolute comparison is done in sat/kwu
118+
/// space (`sats * 250 ≥ floor * txSize`) to avoid float rounding.
119+
bool aboveMinRelay({int? txSize, int? floorSatPerKwu}) {
120+
final floor = floorSatPerKwu ?? minRelaySatPerKwu;
121+
return switch (this) {
122+
RelativeFee(:final satPerKwu) => satPerKwu >= floor,
123+
AbsoluteFee(:final sats) =>
124+
txSize != null && txSize > 0 && (sats * 250) >= (floor * txSize),
125+
};
126+
}
117127
}
118128

119129
@freezed
@@ -122,13 +132,21 @@ abstract class FeeOptions with _$FeeOptions {
122132
required NetworkFee fastest,
123133
required NetworkFee economic,
124134
required NetworkFee slow,
135+
136+
/// The network's current relay floor as a rate — mempool's `minimumFee`
137+
/// clamped up to the static 0.1 sat/vByte safety floor. Validation gates
138+
/// (custom-fee field, commit gates) reject anything below this so the app
139+
/// never builds a tx the network won't relay, even during congestion when
140+
/// `minimumFee` rises above 0.1. A pure rate, never converted to absolute.
141+
required RelativeFee minRelay,
125142
}) = _FeeOptions;
126143
const FeeOptions._();
127144

128145
FeeOptions toAbsolute(int vsize) => FeeOptions(
129146
fastest: fastest.toAbsolute(vsize),
130147
economic: economic.toAbsolute(vsize),
131148
slow: slow.toAbsolute(vsize),
149+
minRelay: minRelay,
132150
);
133151

134152
FeeOptions toRelative(int vsize) {
@@ -144,6 +162,7 @@ abstract class FeeOptions with _$FeeOptions {
144162
fastest: asRelative(fastest),
145163
economic: asRelative(economic),
146164
slow: asRelative(slow),
165+
minRelay: minRelay,
147166
);
148167
}
149168
}

lib/core/widgets/fees/custom_fee_list_item.dart

Lines changed: 34 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ class CustomFeeListItem extends StatefulWidget {
5050
required this.unselectedIconColor,
5151
required this.onCommit,
5252
this.onArm,
53+
this.onDisarm,
5354
this.onPreview,
5455
this.previewFeeSat,
5556
this.previewLoading = false,
@@ -102,6 +103,13 @@ class CustomFeeListItem extends StatefulWidget {
102103
/// Idempotent on the caller side. Ignored in RBF mode.
103104
final void Function(NetworkFee fee)? onArm;
104105

106+
/// Modal mode only. Called when the field is cleared without a valid
107+
/// replacement — the abs/rel toggle is flipped (resets the input) or the
108+
/// text is emptied/invalid. Rolls back any armed custom selection so a
109+
/// stale pre-clear value can't be committed when the sheet is dismissed.
110+
/// Ignored in RBF mode (nothing is armed there).
111+
final VoidCallback? onDisarm;
112+
105113
/// When false, hide the absolute/relative toggle. Input is treated as
106114
/// relative (sat/vByte) only. RBF passes false — its fee API is
107115
/// rate-only.
@@ -239,6 +247,11 @@ class _CustomFeeListItemState extends State<CustomFeeListItem> {
239247
setState(() => _isAbsolute = newValue);
240248
_controller.clear();
241249
_customFee = null;
250+
_previewDebounce?.cancel();
251+
// The field reset to empty — drop any armed custom selection so a stale
252+
// pre-toggle value can't be committed on dismissal (modal mode only;
253+
// RBF commits per-keystroke and has nothing armed).
254+
if (!widget.commitOnChange) widget.onDisarm?.call();
242255
}
243256

244257
void _onValueChanged(String text) {
@@ -255,7 +268,12 @@ class _CustomFeeListItemState extends State<CustomFeeListItem> {
255268
// the user why nothing's getting committed; modal-mode does
256269
// the same gating in [SendCubit.finalizeArmedCustomFee] /
257270
// [TransferBloc._onCustomFeeFinalized] via aboveMinRelay.
258-
if (!fee.aboveMinRelay(txSize: widget.txSize)) return;
271+
if (!fee.aboveMinRelay(
272+
txSize: widget.txSize,
273+
floorSatPerKwu: widget.feePresets?.minRelay.satPerKwu,
274+
)) {
275+
return;
276+
}
259277
widget.onCommit(fee);
260278
} else {
261279
// Modal mode: arm immediately for visual selection (cheap —
@@ -273,6 +291,9 @@ class _CustomFeeListItemState extends State<CustomFeeListItem> {
273291
} else {
274292
setState(() => _customFee = null);
275293
_previewDebounce?.cancel();
294+
// Empty/invalid input — disarm so dismissal rolls back to the prior
295+
// selection instead of committing the last valid armed value.
296+
if (!widget.commitOnChange) widget.onDisarm?.call();
276297
}
277298
}
278299

@@ -333,22 +354,25 @@ class _CustomFeeListItemState extends State<CustomFeeListItem> {
333354
economicKwu == null ||
334355
slowKwu == null)
335356
? ''
336-
: 'Estimated delivery ~ ${customKwu >= fastestKwu
357+
: '${context.loc.sendEstimatedDelivery}${customKwu >= fastestKwu
337358
? context.loc.sendEstimatedDelivery10Minutes
338359
: customKwu >= economicKwu
339360
? context.loc.sendEstimatedDelivery10to30Minutes
340361
: customKwu >= slowKwu
341362
? context.loc.sendEstimatedDeliveryHours
342363
: context.loc.sendEstimatedDeliveryHoursToDays}';
343364

344-
// Fee-rate guards. Single source for the floor is
345-
// NetworkFeeRelayPolicy.minRelaySatPerVbyte (= 0.1, Bitcoin Core's
346-
// lowest sensible policy and Liquid's minrelayfee). Below 1 sat/vByte
347-
// we still warn the tx may take longer to confirm and may not
348-
// propagate to every node.
349-
final bool belowFloor =
350-
customRate != null &&
351-
customRate < NetworkFeeRelayPolicy.minRelaySatPerVbyte;
365+
// Fee-rate guards. The floor is the live network minimum carried by
366+
// [FeeOptions.minRelay] (mempool's `minimumFee` clamped up to the static
367+
// 0.1 sat/vByte safety floor), so under congestion the field rejects
368+
// rates the network won't relay even though they clear the 0.1 constant.
369+
// Falls back to the static 0.1 when no presets are loaded yet (e.g. RBF).
370+
// Below 1 sat/vByte (but at/above the floor) we still warn the tx may
371+
// take longer to confirm and may not propagate to every node.
372+
final double floorSatPerVbyte =
373+
feeOptions?.minRelay.satPerVbyte ??
374+
NetworkFeeRelayPolicy.minRelaySatPerVbyte;
375+
final bool belowFloor = customRate != null && customRate < floorSatPerVbyte;
352376
final bool subOneSatPerVbyte =
353377
customRate != null && customRate < 1.0 && !belowFloor;
354378

lib/core/widgets/fees/fee_modal_controller.dart

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,12 @@ abstract class FeeModalActions {
5656
/// dismisses without typing a valid value.
5757
void armCustomFee(NetworkFee fee);
5858

59+
/// Rolls back an active arm to the pre-arm selection immediately —
60+
/// dispatched by the custom-fee field when it's cleared (abs/rel toggle
61+
/// flipped or text emptied) so a stale armed value can't survive to
62+
/// dismissal. No-op when nothing is armed.
63+
void disarmCustomFee();
64+
5965
/// Called on modal dismissal. Commits the typed value (rebuilding
6066
/// the tx) when it clears `NetworkFeeRelayPolicy.minRelay`;
6167
/// otherwise rolls back to the pre-arm selection.

lib/core/widgets/fees/fee_options_modal.dart

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,7 @@ class _FeeOptionsModalState extends State<FeeOptionsModal> {
126126
previewLoading: snapshot.feePreviewCache.customLoading,
127127
focusNode: _customFeeNode,
128128
onArm: widget.actions.armCustomFee,
129+
onDisarm: widget.actions.disarmCustomFee,
129130
onPreview: widget.actions.requestCustomFeePreview,
130131
// Modal mode: the parent screen runs
131132
// [FeeModalActions.finalizeArmedCustomFee] on
@@ -157,6 +158,7 @@ class _PresetList extends StatelessWidget {
157158
// (pre-existing l10n debt).
158159
final items = [
159160
_presetItem(
161+
context: context,
160162
title: FeeSelection.fastest.title(),
161163
description: context.loc.sendEstimatedDelivery10Minutes,
162164
rate: snapshot.feePresets?.fastest,
@@ -166,6 +168,7 @@ class _PresetList extends StatelessWidget {
166168
fiatCurrencyCode: snapshot.fiatCurrencyCode,
167169
),
168170
_presetItem(
171+
context: context,
169172
title: FeeSelection.economic.title(),
170173
description: context.loc.sendEstimatedDelivery10to30Minutes,
171174
rate: snapshot.feePresets?.economic,
@@ -175,6 +178,7 @@ class _PresetList extends StatelessWidget {
175178
fiatCurrencyCode: snapshot.fiatCurrencyCode,
176179
),
177180
_presetItem(
181+
context: context,
178182
title: FeeSelection.slow.title(),
179183
description: context.loc.sendEstimatedDeliveryHours,
180184
rate: snapshot.feePresets?.slow,
@@ -195,6 +199,7 @@ class _PresetList extends StatelessWidget {
195199
/// hold a fee yet, [SelectableListItem.isSubtitle2Loading] is true so
196200
/// the row renders a shimmer instead of an empty subtitle.
197201
SelectableListItem _presetItem({
202+
required BuildContext context,
198203
required String title,
199204
required String description,
200205
required NetworkFee? rate,
@@ -212,7 +217,7 @@ SelectableListItem _presetItem({
212217
isSubtitle2Loading: true,
213218
);
214219
}
215-
final rateLabel = '${rate.value} sats/vB';
220+
final rateLabel = '${rate.value} ${context.loc.sendSatsPerVB}';
216221
final previewFeeSat = slot.feeSat;
217222
if (previewFeeSat == null) {
218223
return SelectableListItem(
@@ -232,7 +237,8 @@ SelectableListItem _presetItem({
232237
title: title,
233238
subtitle1: description,
234239
subtitle2:
235-
'$rateLabel ~ ${FormatAmount.satsApprox(previewFeeSat)} sats$fiatPart',
240+
'$rateLabel ~ ${FormatAmount.satsApprox(previewFeeSat)} '
241+
'${context.loc.sendSats}$fiatPart',
236242
);
237243
}
238244

lib/features/send/presentation/bloc/send_cubit.dart

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1278,6 +1278,7 @@ class SendCubit extends Cubit<SendState>
12781278
/// Rolls back `selectedFeeOption` and `customFee` to the values snapshotted
12791279
/// by [armCustomFee], if the arm is still active. No-op once cleared
12801280
/// (the user picked a preset, which fires [feeOptionSelected]).
1281+
@override
12811282
void disarmCustomFee() {
12821283
if (state.armPriorSelection == null) return;
12831284
emit(
@@ -1303,7 +1304,11 @@ class SendCubit extends Cubit<SendState>
13031304
if (state.armPriorSelection == null) return;
13041305
final fee = state.customFee;
13051306
final txSize = state.bitcoinTxSize ?? 140;
1306-
if (fee != null && fee.aboveMinRelay(txSize: txSize)) {
1307+
if (fee != null &&
1308+
fee.aboveMinRelay(
1309+
txSize: txSize,
1310+
floorSatPerKwu: state.bitcoinFeesList?.minRelay.satPerKwu,
1311+
)) {
13071312
await customFeesChanged(fee);
13081313
} else {
13091314
disarmCustomFee();
@@ -1635,7 +1640,11 @@ class SendCubit extends Cubit<SendState>
16351640
? (
16361641
unsignedPsbt: cachedSlot.unsignedPsbt!,
16371642
txSize: cachedSlot.txSize!,
1638-
isToSelf: false,
1643+
// The cached PSBT was built for the same address, so the
1644+
// to-self determination is invariant — preserve it rather
1645+
// than dropping it to false (which would flip the "to self"
1646+
// badge and mis-gate payjoin on a self-send).
1647+
isToSelf: state.isToSelf ?? false,
16391648
)
16401649
: await _prepareBitcoinSendUsecase.execute(
16411650
walletId: state.selectedWallet!.id,

0 commit comments

Comments
 (0)