Skip to content

Commit b56161c

Browse files
markettesRodriFS
markettes
and
RodriFS
authored
[GEN-1699] Fix critical bug in RBF changeless withdrawals (#425)
* fix: critical bug in rbf changeless withdrawals * Fixed modal bumping on signature collection instead of after job creation * added description prefix to bumps * Fixed creation of withdrawals not calling nbxplorer to check balance * Fix bump when utxo is already confirmed --------- Co-authored-by: Rodrigo <[email protected]>
1 parent 35353a2 commit b56161c

File tree

4 files changed

+185
-91
lines changed

4 files changed

+185
-91
lines changed

‎src/Helpers/CustomExceptions.cs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,3 +33,8 @@ public class NotEnoughBalanceInWalletException : Exception
3333
{
3434
public NotEnoughBalanceInWalletException(string? message = null): base(message) {}
3535
}
36+
37+
public class BumpingException : Exception
38+
{
39+
public BumpingException(string? message = null): base(message) {}
40+
}

‎src/Pages/Withdrawals.razor‎

Lines changed: 113 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -468,6 +468,7 @@
468468
@inject INBXplorerService NBXplorerService
469469
@inject IFMUTXORepository FMUTXORepository
470470
@inject IWalletWithdrawalRequestDestinationRepository WalletWithdrawalRequestDestinationRepository
471+
@inject ILogger<Withdrawals> Logger
471472
@code {
472473

473474
[CascadingParameter] private ApplicationUser? LoggedUser { get; set; }
@@ -628,8 +629,6 @@
628629

629630
arg.Item.Wallet = null;
630631
var addResult = await WalletWithdrawalRequestRepository.AddAsync(arg.Item);
631-
632-
633632
if (addResult.Item1)
634633
{
635634
ToastService.ShowSuccess("Success");
@@ -707,8 +706,6 @@
707706
if (await ValidateBalance(arg)) return;
708707

709708
var updateResult = WalletWithdrawalRequestRepository.Update(arg.Item);
710-
711-
712709
if (updateResult.Item1)
713710
{
714711
ToastService.ShowSuccess("Success");
@@ -779,16 +776,11 @@
779776

780777
private async Task ShowApprovalModal(WalletWithdrawalRequest walletWithdrawalRequest)
781778
{
782-
_selectedRequest = walletWithdrawalRequest;
779+
_selectedRequest = await WalletWithdrawalRequestRepository.GetById(walletWithdrawalRequest.Id);
783780

784781
//PSBT Generation
785782
try
786783
{
787-
if (_selectedRequest.BumpingWalletWithdrawalRequestId != null)
788-
{
789-
await CopyBumpedRequestData(_selectedRequest);
790-
}
791-
792784
var templatePSBT = await BitcoinService.GenerateTemplatePSBT(walletWithdrawalRequest);
793785

794786
//TODO Save template PSBT (?)
@@ -799,6 +791,18 @@
799791
{
800792
ToastService.ShowError("No UTXOs available for withdrawals were found for this wallet");
801793
}
794+
catch (BumpingException e)
795+
{
796+
ToastService.ShowError(e.Message);
797+
}
798+
catch (ShowToUserException e)
799+
{
800+
ToastService.ShowError(e.Message);
801+
_selectedRequest.Status = WalletWithdrawalRequestStatus.Failed;
802+
WalletWithdrawalRequestRepository.Update(_selectedRequest);
803+
await GetData();
804+
StateHasChanged();
805+
}
802806
catch (Exception)
803807
{
804808
ToastService.ShowError("Error while generating PSBT template for the request");
@@ -904,7 +908,11 @@
904908
_selectedRequest = await WalletWithdrawalRequestRepository.GetById(_selectedRequest.Id);
905909
if (_selectedRequest != null)
906910
{
907-
CreateJob();
911+
await CreateJob();
912+
if (_selectedRequest?.BumpingWalletWithdrawalRequestId != null)
913+
{
914+
await UpdateBumpWalletWithdrawalRequest();
915+
}
908916
}
909917
}
910918
catch
@@ -929,7 +937,7 @@
929937
}
930938
else
931939
{
932-
ToastService.ShowSuccess("Error while saving the signature");
940+
ToastService.ShowError("Error while saving the signature");
933941
}
934942

935943
await _psbtSignRef.HideModal();
@@ -942,20 +950,34 @@
942950
{
943951
_selectedMempoolRecommendedFeesType = newRequest.MempoolRecommendedFeesType;
944952
_customSatPerVbAmount = (long)newRequest.CustomFeeRate;
953+
_selectedRequest = newRequest;
945954
if (newRequest.Wallet.IsHotWallet)
946955
{
947956
await GetData();
948-
_selectedRequest = newRequest;
949957
await _confirmationModal.ShowModal();
950958
return;
951959
}
952960

953961
newRequest.Wallet = null;
954-
var addResult = await WalletWithdrawalRequestRepository.AddAsync(newRequest);
955962

963+
try {
964+
if (_selectedRequest.Id == 0)
965+
{
966+
var createWithdrawalResult = await WalletWithdrawalRequestRepository.AddAsync(_selectedRequest);
967+
if (!createWithdrawalResult.Item1)
968+
{
969+
throw new ShowToUserException(createWithdrawalResult.Item2);
970+
}
971+
}
972+
else
973+
{
974+
var updateResult = WalletWithdrawalRequestRepository.Update(_selectedRequest);
975+
if (!updateResult.Item1)
976+
{
977+
throw new ShowToUserException(updateResult.Item2);
978+
}
979+
}
956980

957-
if (addResult.Item1)
958-
{
959981
ToastService.ShowSuccess("Success");
960982
await GetData();
961983

@@ -966,11 +988,16 @@
966988

967989
_utxoSelectorModalRef.ClearModal();
968990
}
969-
else
991+
catch (ShowToUserException e)
970992
{
971-
ToastService.ShowError("Something went wrong");
993+
ToastService.ShowError(e.Message);
972994
_userPendingRequests.Remove(newRequest);
973995
}
996+
catch (Exception e)
997+
{
998+
Logger.LogError("Error creating a new withdrawal request for bumping fee: {Error}", e.Message);
999+
ToastService.ShowError("Something went wrong");
1000+
}
9741001
}
9751002

9761003
private async Task Bumpfee(WalletWithdrawalRequest request)
@@ -991,7 +1018,6 @@
9911018
{
9921019
if (newRequest != null && _bumpfeeRef != null)
9931020
{
994-
newRequest.WalletId = _selectedRequest.WalletId;
9951021
newRequest.Wallet = await WalletRepository.GetById(newRequest.WalletId);
9961022

9971023
// Use either the custom fee rate or the recommended fee rate based on mempool fee type
@@ -1044,6 +1070,7 @@
10441070

10451071
await Bumpfee(newRequest);
10461072
await _bumpfeeRef.HideModal();
1073+
StateHasChanged();
10471074
}
10481075
else
10491076
{
@@ -1081,12 +1108,15 @@
10811108
_selectedRequest.Status = (WalletWithdrawalRequestStatus) _rejectCancelStatus;
10821109

10831110
var updateResult = WalletWithdrawalRequestRepository.Update(_selectedRequest);
1084-
1085-
if (updateResult.Item1 == false)
1111+
if (!updateResult.Item1)
10861112
{
1113+
Logger.LogError("Error while updating the withdrawal request status to cancelled/rejected: {Error}", updateResult.Item2);
10871114
ToastService.ShowError("Something went wrong");
10881115
}
10891116

1117+
// In case that is a bump, we return the old request status to pending confirmation.
1118+
await ResetStatusBumpedIfError(_selectedRequest);
1119+
10901120
await HideRejectCancelModal();
10911121
}
10921122
}
@@ -1151,9 +1181,45 @@
11511181

11521182
private async Task CloseConfirmationModal()
11531183
{
1184+
if (_selectedRequest != null)
1185+
{
1186+
_selectedRequest.Status = WalletWithdrawalRequestStatus.Cancelled;
1187+
_selectedRequest.RejectCancelDescription = "User cancelled the operation";
1188+
var updateResult = WalletWithdrawalRequestRepository.Update(_selectedRequest);
1189+
if (!updateResult.Item1)
1190+
{
1191+
Logger.LogError("Error while updating the withdrawal request status to cancelled: {Error}", updateResult.Item2);
1192+
ToastService.ShowError("Something went wrong");
1193+
}
1194+
StateHasChanged();
1195+
}
11541196
await _confirmationModal.CloseModal();
11551197
}
11561198

1199+
private async Task UpdateBumpWalletWithdrawalRequest()
1200+
{
1201+
if (_selectedRequest == null) throw new BumpingException("Invalid request");
1202+
1203+
var outpoints = _selectedRequest.UTXOs.Select(u => OutPoint.Parse($"{u.TxId}:{u.OutputIndex}")).ToList();
1204+
_selectedUTXOs = await CoinSelectionService.GetUTXOsByOutpointAsync(_selectedRequest.Wallet.GetDerivationStrategy()!, outpoints);
1205+
1206+
// Update the status of the old withdrawal request
1207+
if (_selectedRequest.BumpingWalletWithdrawalRequestId != null)
1208+
{
1209+
_selectedRequest.BumpingWalletWithdrawalRequest = await WalletWithdrawalRequestRepository.GetById(_selectedRequest.BumpingWalletWithdrawalRequestId.Value);
1210+
if (_selectedRequest.BumpingWalletWithdrawalRequest == null)
1211+
{
1212+
throw new BumpingException("Could not find bumping withdrawal request");
1213+
}
1214+
_selectedRequest.BumpingWalletWithdrawalRequest.Status = WalletWithdrawalRequestStatus.Bumped;
1215+
var updateResultOldWithdrawal = WalletWithdrawalRequestRepository.Update(_selectedRequest.BumpingWalletWithdrawalRequest);
1216+
if (!updateResultOldWithdrawal.Item1)
1217+
{
1218+
throw new BumpingException("Something went wrong");
1219+
}
1220+
}
1221+
}
1222+
11571223
private async Task SubmitConfirmationModal()
11581224
{
11591225
async Task CleanUp(string errorMessage)
@@ -1170,20 +1236,31 @@
11701236
{
11711237
try
11721238
{
1173-
_selectedRequest.Wallet = null;
1239+
_selectedRequest.Wallet = await WalletRepository.GetById(_selectedRequest.WalletId);
11741240
_selectedRequest.WithdrawAllFunds = _isCheckedAllFunds || _amount == _selectedRequestWalletBalance;
11751241
_selectedRequest.MempoolRecommendedFeesType = _selectedMempoolRecommendedFeesType;
11761242
_selectedRequest.CustomFeeRate = _customSatPerVbAmount;
11771243

1178-
var createWithdrawalResult = await WalletWithdrawalRequestRepository.AddAsync(_selectedRequest);
1179-
if (!createWithdrawalResult.Item1)
1244+
if (_selectedRequest.Id == 0)
11801245
{
1181-
throw new ShowToUserException(createWithdrawalResult.Item2);
1246+
var createWithdrawalResult = await WalletWithdrawalRequestRepository.AddAsync(_selectedRequest);
1247+
if (!createWithdrawalResult.Item1)
1248+
{
1249+
throw new ShowToUserException(createWithdrawalResult.Item2);
1250+
}
1251+
}
1252+
else
1253+
{
1254+
var updateResult = WalletWithdrawalRequestRepository.Update(_selectedRequest);
1255+
if (!updateResult.Item1)
1256+
{
1257+
throw new ShowToUserException(updateResult.Item2);
1258+
}
11821259
}
11831260

11841261
if (_selectedRequest.BumpingWalletWithdrawalRequestId != null)
11851262
{
1186-
await CopyBumpedRequestData(_selectedRequest);
1263+
await UpdateBumpWalletWithdrawalRequest();
11871264
}
11881265

11891266
_selectedRequest.Changeless = _selectedUTXOs.Count > 0;
@@ -1205,7 +1282,6 @@
12051282
};
12061283

12071284
var addResult = await WalletWithdrawalRequestPsbtRepository.AddAsync(walletWithdrawalRequestPsbt);
1208-
12091285
if (!addResult.Item1)
12101286
{
12111287
throw new ShowToUserException("Error while saving the signature");
@@ -1232,17 +1308,23 @@
12321308
}
12331309
catch (NoUTXOsAvailableException e)
12341310
{
1235-
CleanUp("No UTXOs available for withdrawals were found for this wallet");
1311+
await CleanUp("No UTXOs available for withdrawals were found for this wallet");
12361312
await ResetStatusBumpedIfError(_selectedRequest);
12371313
}
12381314
catch (ShowToUserException e)
12391315
{
1240-
CleanUp(e.Message);
1316+
await CleanUp(e.Message);
1317+
await ResetStatusBumpedIfError(_selectedRequest);
1318+
}
1319+
catch (BumpingException e)
1320+
{
1321+
await CleanUp(e.Message);
12411322
await ResetStatusBumpedIfError(_selectedRequest);
12421323
}
1243-
catch
1324+
catch (Exception e)
12441325
{
1245-
CleanUp("Something went wrong");
1326+
Logger.LogError(e, "Error while creating withdrawal");
1327+
await CleanUp("Something went wrong");
12461328
await ResetStatusBumpedIfError(_selectedRequest);
12471329
}
12481330
}
@@ -1390,56 +1472,4 @@
13901472

13911473
return false;
13921474
}
1393-
1394-
private async Task CopyBumpedRequestData(WalletWithdrawalRequest request)
1395-
{
1396-
WalletWithdrawalRequest originalRequest = await WalletWithdrawalRequestRepository.GetById((int)request.BumpingWalletWithdrawalRequestId);
1397-
if (originalRequest == null)
1398-
{
1399-
throw new ShowToUserException("Original withdrawal request not found");
1400-
}
1401-
1402-
// Get destinations from the original request
1403-
List<WalletWithdrawalRequestDestination> originalDestinations = await WalletWithdrawalRequestDestinationRepository.GetByWalletWithdrawalRequestId(originalRequest.Id);
1404-
if (originalDestinations == null || originalDestinations.Count == 0)
1405-
{
1406-
throw new ShowToUserException("Original withdrawal request destinations not found");
1407-
}
1408-
1409-
request.WalletWithdrawalRequestDestinations = originalDestinations.Select(d => new WalletWithdrawalRequestDestination
1410-
{
1411-
Address = d.Address,
1412-
Amount = d.Amount,
1413-
WalletWithdrawalRequestId = request.Id
1414-
}).ToList();
1415-
1416-
// Get utxos from the original request
1417-
List<FMUTXO> mUTXOs = await FMUTXORepository.GetLockedUTXOsByWithdrawalId(originalRequest.Id);
1418-
1419-
request.UTXOs = mUTXOs;
1420-
List<OutPoint> outpoints = new List<OutPoint>();
1421-
mUTXOs.ForEach(u => {
1422-
OutPoint o;
1423-
if (OutPoint.TryParse($"{u.TxId}:{u.OutputIndex}", out o))
1424-
outpoints.Add(o);
1425-
else
1426-
throw new FormatException("Failed to parse OutPoint from TxId and OutputIndex");
1427-
});
1428-
_selectedUTXOs = await CoinSelectionService.GetUTXOsByOutpointAsync(originalRequest.Wallet.GetDerivationStrategy(), outpoints);
1429-
originalRequest.Status = WalletWithdrawalRequestStatus.Bumped;
1430-
1431-
var updateResult = WalletWithdrawalRequestRepository.Update(request);
1432-
if (!updateResult.Item1)
1433-
{
1434-
ToastService.ShowError("Something went wrong");
1435-
return;
1436-
}
1437-
1438-
updateResult = WalletWithdrawalRequestRepository.Update(originalRequest);
1439-
if (!updateResult.Item1)
1440-
{
1441-
ToastService.ShowError("Something went wrong");
1442-
return;
1443-
}
1444-
}
14451475
}

‎src/Services/BitcoinService.cs‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,13 @@ public async Task<PSBT> GenerateTemplatePSBT(WalletWithdrawalRequest walletWithd
177177
await _coinSelectionService.GetLockedUTXOsForRequest(walletWithdrawalRequest,
178178
BitcoinRequestType.WalletWithdrawal);
179179

180+
// Edge case: If you bumped a multisig transaction and a block was mined in between, the utxo is now unlocked, so we need to fail here
181+
// So a new withdrawal isn't performed with a new utxo
182+
if (previouslyLockedUTXOs.Count == 0 && walletWithdrawalRequest.BumpingWalletWithdrawalRequestId != null)
183+
{
184+
throw new ShowToUserException($"Cannot generate a template PSBT for an already confirmed bumped transaction. The UTXO for request {walletWithdrawalRequest.BumpingWalletWithdrawalRequestId} is already confirmed");
185+
}
186+
180187

181188
var availableUTXOs = previouslyLockedUTXOs.Count > 0
182189
? previouslyLockedUTXOs

0 commit comments

Comments
 (0)