What changed, and why it matters
This commit changes how the Skylight Wallet app estimates Monero transaction fees. Previously, the app calculated fees by actually building full draft transactions and then reusing those draft transactions when the user pressed send. Now it uses a dedicated native fee-estimation function and always builds the real transaction only when the user confirms the send. The change also switches the underlying Monero library from a personal repository (vtnerd/monero_c) to an organization-owned fork (magicgrants/monero_c). The main security-relevant effect is reducing the risk that a stale or reused draft transaction gets sent accidentally, and it removes a retry loop that could have produced misleading fee information. There is no explicit security bug fixed in the diff itself, so the security relevance is moderate and inferred.
Treat this as a defensive hardening change rather than an urgent security patch. Review the new estimateFee() implementation for correct failure handling and ensure the magicgrants/monero_c fork is kept in sync with upstream security fixes. Verify that the deprecated Wallet_estimateTransactionFee FFI call is replaced when the upstream API changes. Users should update to the version containing this commit to benefit from more reliable fee estimates and reduced transaction-reuse risk.
Security signals we found
Eliminates reuse of cached pending transactions for fee display and later sending, reducing risk of stale/fee-mismatched transaction submission
Removes a 10-attempt retry loop around createTx() that silently swallowed 'Unlocked funds too low' errors and could present inaccurate fee data
Adds native fee estimation via deprecated FFI API with null/0 failure handling
Switches upstream Monero C library source from vtnerd/monero_c to magicgrants/monero_c fork
Adds UI '~' prefix to estimated fees to signal they are approximate
No explicit security disclosure, CVE, or advisory referenced in commit or supplied materials
Evidence from the diff
The patch replaces a fee-discovery mechanism that constructed real MoneroPendingTransaction objects for three priority levels via wallet.createTx() and cached them for later reuse in _onConfirmSend(). It introduces WalletModel.estimateFee(), which calls monero.Wallet_estimateTransactionFee() in an Isolate and returns an int fee in piconero. The UI now stores List
Changed components
lib/models/wallet_model.dartlib/screens/send.dartlib/widgets/monero_amount.dartmonero_c submodule / pubspec dependencyInspect captured patch +74 / −77
diff --git a/.gitmodules b/.gitmodules
index 291ddc6..cfe02a0 100644
--- a/.gitmodules
+++ b/.gitmodules
@@ -1,3 +1,3 @@
[submodule "monero_c"]
path = monero_c
- url = https://github.com/vtnerd/monero_c.git
+ url = https://github.com/magicgrants/monero_c
diff --git a/lib/models/wallet_model.dart b/lib/models/wallet_model.dart
index b962ac8..b00750b 100644
--- a/lib/models/wallet_model.dart
+++ b/lib/models/wallet_model.dart
@@ -1366,6 +1366,39 @@ class WalletModel with ChangeNotifier {
return subaddress;
}
+ /// Estimates the network fee (in piconero) for a send at [priority] via the
+ /// native estimator. Returns null on failure or when fee info isn't cached yet
+ Future<int?> estimateFee(
+ String destinationAddress,
+ double amount, {
+ int priority = 0,
+ String? amountText,
+ }) async {
+ if (_w2Wallet == null) return null;
+
+ final amountInt = amountText != null
+ ? decimalToBaseUnits(amountText, consts.moneroDecimals).toInt()
+ : _w2Wallet!.amountFromDouble(amount);
+ final walletFfiAddr = _w2Wallet!.ffiAddress();
+
+ try {
+ final fee = await Isolate.run(() {
+ // ignore: deprecated_member_use
+ return monero.Wallet_estimateTransactionFee(
+ Pointer.fromAddress(walletFfiAddr),
+ dstAddr: [destinationAddress],
+ amounts: [amountInt],
+ pendingTransactionPriority: priority,
+ );
+ });
+ // 0 = backend couldn't estimate (never a real fee).
+ return fee > 0 ? fee : null;
+ } catch (e) {
+ log(LogLevel.warn, 'estimateFee failed: $e');
+ return null;
+ }
+ }
+
Future<MoneroPendingTransaction> createTx(
String destinationAddress,
double amount,
diff --git a/lib/screens/send.dart b/lib/screens/send.dart
index 50342f4..58a776f 100644
--- a/lib/screens/send.dart
+++ b/lib/screens/send.dart
@@ -4,8 +4,6 @@ import 'package:flutter/material.dart';
import 'package:flutter/services.dart';
import 'package:flutter_svg/flutter_svg.dart';
import 'package:provider/provider.dart';
-// ignore: implementation_imports
-import 'package:monero/src/monero.dart';
import 'package:skylight_wallet/consts.dart' as consts;
import 'package:skylight_wallet/l10n/app_localizations.dart';
import 'package:skylight_wallet/models/fiat_rate_model.dart';
@@ -40,7 +38,7 @@ class _SendScreenState extends State<SendScreen> {
final _amountController = TextEditingController(text: '');
bool _isSweepAll = false;
Contact? _selectedContact;
- List<MoneroPendingTransaction?>? _fees;
+ List<int?>? _fees; // estimated fee (piconero) per priority; null = estimate failed
int _selectedPriority = 1; // 0=Low, 1=Normal, 2=High
int _feeCalculationCounter = 0; // Track the latest fee calculation request
String _lastFeeFetchKey = '';
@@ -257,39 +255,6 @@ class _SendScreenState extends State<SendScreen> {
return true;
}
- Future<MoneroPendingTransaction?> _createTxForPriority(
- String destinationAddress,
- double amount,
- int priority, {
- String? amountText,
- }) async {
- final wallet = Provider.of<WalletModel>(context, listen: false);
- const maxRetries = 10;
-
- for (int i = 0; i < maxRetries; i++) {
- try {
- final tx = await wallet.createTx(
- destinationAddress,
- amount,
- _isSweepAll,
- priority: priority,
- amountText: amountText,
- );
- return tx;
- } catch (error) {
- if (error.toString().contains('Unlocked funds too low')) {
- return null;
- }
-
- if (i == maxRetries - 1) {
- rethrow;
- }
- }
- }
-
- throw Exception('Failed to create fee priority transaction after $maxRetries retries');
- }
-
Future<void> _calculateFees() async {
final feeFetchKey = '${_destinationAddressController.text}-${_amountController.text}';
@@ -300,6 +265,7 @@ class _SendScreenState extends State<SendScreen> {
_lastFeeFetchKey = feeFetchKey;
final i18n = AppLocalizations.of(context)!;
+ final wallet = Provider.of<WalletModel>(context, listen: false);
// Increment counter to mark this as the latest request
_feeCalculationCounter++;
@@ -315,16 +281,17 @@ class _SendScreenState extends State<SendScreen> {
final amount = double.parse(amountText);
try {
- final txs = await Future.wait([
- _createTxForPriority(destinationAddress, amount, 1, amountText: amountText),
- _createTxForPriority(destinationAddress, amount, 2, amountText: amountText),
- _createTxForPriority(destinationAddress, amount, 3, amountText: amountText),
+ // Estimate the fee per priority natively (no full tx build).
+ final fees = await Future.wait([
+ wallet.estimateFee(destinationAddress, amount, priority: 1, amountText: amountText),
+ wallet.estimateFee(destinationAddress, amount, priority: 2, amountText: amountText),
+ wallet.estimateFee(destinationAddress, amount, priority: 3, amountText: amountText),
]);
// Only update state if this is still the latest request
if (currentRequest == _feeCalculationCounter && mounted) {
setState(() {
- _fees = txs;
+ _fees = fees;
_isLoadingFees = false;
// If there is not enough balance for the selected priority,
@@ -386,26 +353,15 @@ class _SendScreenState extends State<SendScreen> {
}
try {
- MoneroPendingTransaction tx;
-
- // Check if we can reuse a cached transaction
- final currentFeeFetchKey = '${_destinationAddressController.text}-${_amountController.text}';
- final cachedTx = _fees != null && _fees!.length > _selectedPriority
- ? _fees![_selectedPriority]
- : null;
-
- if (currentFeeFetchKey == _lastFeeFetchKey && cachedTx != null) {
- tx = cachedTx;
- } else {
- // Create a new transaction if cached one is not available
- tx = await wallet.createTx(
- destinationAddress,
- amount,
- _isSweepAll,
- priority: _selectedPriority + 1,
- amountText: _amountController.text,
- );
- }
+ // Build the real transaction for the selected priority (fees shown on the
+ // screen are estimates, not tx objects, so always construct here).
+ final tx = await wallet.createTx(
+ destinationAddress,
+ amount,
+ _isSweepAll,
+ priority: _selectedPriority + 1,
+ amountText: _amountController.text,
+ );
setState(() {
_isLoading = false;
@@ -715,8 +671,8 @@ class _SendScreenState extends State<SendScreen> {
)
else if (_fees != null && _fees!.length > _selectedPriority)
() {
- final selectedTx = _fees![_selectedPriority];
- if (selectedTx != null) {
+ final fee = _fees![_selectedPriority];
+ if (fee != null) {
return Row(
spacing: 8,
children: [
@@ -730,8 +686,9 @@ class _SendScreenState extends State<SendScreen> {
height: 14,
),
MoneroAmount(
- amount: doubleAmountFromInt(selectedTx.fee()),
+ amount: doubleAmountFromInt(fee),
maxFontSize: 14,
+ prefix: '~',
),
],
),
@@ -910,7 +867,7 @@ class _ContactPickerDialogState extends State<_ContactPickerDialog> {
class _PriorityOption extends StatelessWidget {
final String label;
final int priority;
- final List<MoneroPendingTransaction?>? fees;
+ final List<int?>? fees;
final String fiatSymbol;
final double? fiatRate;
final bool isSelected;
@@ -929,8 +886,8 @@ class _PriorityOption extends StatelessWidget {
@override
Widget build(BuildContext context) {
final i18n = AppLocalizations.of(context)!;
- final feeTx = fees?[priority];
- final fee = feeTx != null ? doubleAmountFromInt(feeTx.fee()) : null;
+ final feePiconero = fees?[priority];
+ final fee = feePiconero != null ? doubleAmountFromInt(feePiconero) : null;
final currentFiatRate = fiatRate;
return InkWell(
@@ -983,7 +940,7 @@ class _PriorityOption extends StatelessWidget {
),
),
SvgPicture.asset('assets/icons/monero.svg', width: 14, height: 14),
- MoneroAmount(amount: fee, maxFontSize: 14),
+ MoneroAmount(amount: fee, maxFontSize: 14, prefix: '~'),
],
),
if (currentFiatRate != null)
diff --git a/lib/widgets/monero_amount.dart b/lib/widgets/monero_amount.dart
index b9b1e49..4e342f9 100644
--- a/lib/widgets/monero_amount.dart
+++ b/lib/widgets/monero_amount.dart
@@ -3,11 +3,13 @@ import 'package:flutter/material.dart';
class MoneroAmount extends StatelessWidget {
final double amount;
final double maxFontSize;
+ final String? prefix;
const MoneroAmount({
super.key,
required this.amount,
required this.maxFontSize,
+ this.prefix,
});
@override
@@ -20,6 +22,11 @@ class MoneroAmount extends StatelessWidget {
mainAxisAlignment: MainAxisAlignment.center,
crossAxisAlignment: CrossAxisAlignment.start,
children: [
+ if (prefix != null)
+ Text(
+ prefix!,
+ style: TextStyle(fontSize: maxFontSize, fontWeight: FontWeight.w700),
+ ),
Text(
biggerSlice,
style: TextStyle(fontSize: maxFontSize, fontWeight: FontWeight.w700),
diff --git a/pubspec.lock b/pubspec.lock
index cc09ae2..53591af 100644
--- a/pubspec.lock
+++ b/pubspec.lock
@@ -197,10 +197,10 @@ packages:
dependency: transitive
description:
name: ffi
- sha256: "289279317b4b16eb2bb7e271abccd4bf84ec9bdcbe999e278a94b804f5630418"
+ sha256: "6d7fd89431262d8f3125e81b50d3847a091d846eafcd4fdb88dd06f36d705a45"
url: "https://pub.dev"
source: hosted
- version: "2.1.4"
+ version: "2.2.0"
file:
dependency: transitive
description:
@@ -665,9 +665,9 @@ packages:
dependency: "direct main"
description:
path: "impls/monero.dart"
- ref: "8de1614734007782bb2701ab16c49adf0df8e850"
- resolved-ref: "8de1614734007782bb2701ab16c49adf0df8e850"
- url: "https://github.com/vtnerd/monero_c"
+ ref: "8c9af02665e0c0c8e2e8dc7022679b2743842ed4"
+ resolved-ref: "8c9af02665e0c0c8e2e8dc7022679b2743842ed4"
+ url: "https://github.com/magicgrants/monero_c"
source: git
version: "0.0.0"
nested:
diff --git a/pubspec.yaml b/pubspec.yaml
index 0f5f1b7..36be841 100644
--- a/pubspec.yaml
+++ b/pubspec.yaml
@@ -56,8 +56,8 @@ dependencies:
monero:
git:
- url: https://github.com/vtnerd/monero_c
- ref: 8de1614734007782bb2701ab16c49adf0df8e850
+ url: https://github.com/magicgrants/monero_c
+ ref: c74f8dfcc07f56720c0239d5829995c8e20033c2
path: impls/monero.dart
openalias_ffi:
path: plugins/openalias_ffi
Why this scored 33/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.