Code Monkey home page Code Monkey logo

Comments (11)

lukasdll avatar lukasdll commented on July 18, 2024 4

I could prevent the bug from appearing by altering NBitcoin FeeRate at FeeRate.cs.

The conversion from decimal to long truncates the decimal portion. This can result in small inaccuracies.

E.g.:
The fee rate per byte would be: 101/70 = 1.442857...
The fee rate per kilobyte would be: 1.442857 * 1000 = 1442.857...
But, when converted to long, the value becomes 1442, effectively rounding down and losing the 0.857 fractional value.

		public FeeRate(Money feePaid, int size)
		{
			if (feePaid is null)
				throw new ArgumentNullException(nameof(feePaid));
			if (feePaid.Satoshi < 0)
				throw new ArgumentOutOfRangeException(nameof(feePaid), "Cannot be less than 0.");
			if (size > 0)
				_FeePerK = (long)Math.Round((decimal)feePaid.Satoshi / size * 1000m, MidpointRounding.AwayFromZero);
			else
				_FeePerK = Money.Zero;
		}

By using Math.Round with MidpointRounding.AwayFromZero, any decimal value from .5 to .999' is always rounded up to the next whole number. Without specifying MidpointRounding.AwayFromZero, and thus using the default MidpointRounding.ToEven (Banker’s Rounding), values ending in .5 are rounded to the nearest even whole number, which may produce discrepancies or undesired results in certain circumstances.

I'm going to test it a bit more and make a pull request to the NBitcoin repo.

from walletwasabi.

nopara73 avatar nopara73 commented on July 18, 2024 1

not enough additional fees to relay; 0.00000152 < 0.00000153

looks like you only need a single satoshi there : )

from walletwasabi.

lukasdll avatar lukasdll commented on July 18, 2024 1

Update: After doing many more tests than the 4 below, I could reproduce the issue, but the conditions to reproduce are yet unclear. Sometimes speeding up a self-transaction works fine others it doesn't.


Detailed explanation:

To try to reproduce the issue, I made 2 different tests on Main and 2 different tests on Testnet, and I could not reproduce any unexpected behavior, but I found a potential improvement (balance check + UX message).

On Main I could replace the transaction without any error related to fees, both using the speed-up and the cancellation.

Main test 1:

  1. Send the transaction with default fees.
    https://mempool.space/tx/8f7b055f8e88cb7a0fe589c585d84b847f75d5ea2df9e1af357fe4366168acdd
  2. Speed-up the transaction (success).
    https://mempool.space/tx/86cc02ec625cef07d3fb009c7f89e851a96a6cd7a1a2500a84e6b916d99126f6
  3. Cancel the transaction (success).
    https://mempool.space/tx/8f7b055f8e88cb7a0fe589c585d84b847f75d5ea2df9e1af357fe4366168acdd

Main test 2:

  1. Send the transaction with lower fees than the default.
    https://mempool.space/tx/69792d8483c7e9009e612fc3efa649e7be0bf430d079e3b1c4297bce825f0051
  2. Cancel the transaction (success).
    https://mempool.space/tx/9ebeda9b6894799ee0b9997b21487fedf15d8c3242aaacf80d1ea8990173b77a

OS: Linux
Wallet version: v2.0.4
Sha256: 1dbc4b531f75504631277898b274d068651fa1ad2cdb7df455d239d15a97abaf Wasabi-2.0.4.deb


Then I tried to reproduce on Testnet with a different OS (Windows).

On Testnet (Test 1), I could obtain the same error message.

Testnet 1:

  1. Send the transaction with low fees (1.36 sats v/B).
    https://mempool.space/testnet/tx/423f4e5fbcd73bf559cd8103682cbf064039404986eac98c5d9a9b909b417dce
  2. Try to speed-up, and it fails: HttpRequestException: BadRequest insufficient fee, rejecting replacement.

Logs:

TransactionBroadcaster.BroadcastTransactionToBackendAsync (94)  Broadcasting with backend...
ERROR  SpeedUpTransactionDialogViewModel.OnSpeedUpTransactionAsync (97)  System.Net.Http.HttpRequestException: Bad Request
insufficient fee, rejecting replacement c727254e9ad92236d999619ebfd702e0722867ada249429e159a8c71feecefbe, not enough additional fees to relay; 0.00000109 < 0.0000011:::01000000000101e6d4345bac49e96736e86ca19c0e35f4cc8b0becd9aa6047ec2ded1c64e0ecea0000000000fdffffff01ab1e000000000000160014f982508ba23df0209c0cf526da53a16b75294d29024730440220510430901a220c843fe7947907e153fdec0070ef38f8b85f8199f7867f53cfce022036f075dbdbe455b7bf29a4548be0701c50b937729da8a79a55e9a9fdfd2754c20121031975815df14a24d9fb3d2e9a2d5d3dd287cfeb9a5be0539f38bc54d207586abc00000000
   at WalletWasabi.Tor.Http.Extensions.HttpResponseMessageExtensions.ThrowRequestExceptionFromContentAsync(HttpResponseMessage me, CancellationToken cancellationToken)
   at WalletWasabi.WebClients.Wasabi.WasabiClient.BroadcastAsync(String hex)
   at WalletWasabi.WebClients.Wasabi.WasabiClient.BroadcastAsync(Transaction transaction)
   at WalletWasabi.WebClients.Wasabi.WasabiClient.BroadcastAsync(SmartTransaction transaction)
   at WalletWasabi.Blockchain.TransactionBroadcasting.TransactionBroadcaster.BroadcastTransactionToBackendAsync(SmartTransaction transaction)
   at WalletWasabi.Blockchain.TransactionBroadcasting.TransactionBroadcaster.SendTransactionAsync(SmartTransaction transaction)
   at WalletWasabi.Fluent.ViewModels.Wallets.Home.History.Features.SpeedUpTransactionDialogViewModel.OnSpeedUpTransactionAsync(BuildTransactionResult boostingTransaction)

Testnet 2:

  1. Send the transaction with low fees (1.37 sats v/B).
    https://mempool.space/testnet/tx/460d4257c0014df4e8635ec210ec88b4ec06ca1dbc414603bd3a99dcc024a6fc
  2. Speed-up the transaction (success).
    https://mempool.space/testnet/tx/328b33799307cdbe6e8e67fff0d1f940127756321cadd0d1768b95a0208771b3

OS: Windows
Wallet version: v2.0.4
Sha256: 97cfc014ab8d735c9530aac74537e322704779c641f35ac2bf21619039536ab2 Wasabi-2.0.4.msi

from walletwasabi.

lukasdll avatar lukasdll commented on July 18, 2024 1

The problem happens when the final fee rate has three decimals. In the previous failing example, the fee rate was: 2.445 Sat/B.

I pulled the "cables" and found a few suspects in the NBitcoin library:

public decimal SatoshiPerByte
{
    get
    {
        return (decimal)_FeePerK.Satoshi / 1000;
    }
}

If _FeePerK.Satoshi has a value that doesn't evenly divide by 1000, we could lose some precision.

And also:

public Money GetFee(int virtualSize)
{
    Money nFee = _FeePerK.Satoshi * virtualSize / 1000;
    if (nFee == Money.Zero)
        nFee = Money.Satoshis(1.0m);
    return nFee;
}

This could also lead to rounding issues, especially if virtualSize doesn't multiply evenly into _FeePerK.Satoshi * 1000.

A quick & easy solution could be to ceil-up the rbfFeeRate amount in RbfTransaction(...) to two decimals.

		var rbfFeeRate = (bestFeeRate is null || bestFeeRate <= minimumRbfFeeRate)
			? minimumRbfFeeRate
			: bestFeeRate;

		decimal originalRbfFeeRateValue = rbfFeeRate.SatoshiPerByte;
		decimal roundedRbfFeeRateValue = Math.Ceiling(originalRbfFeeRateValue * 100) / 100;
		rbfFeeRate = new FeeRate(roundedRbfFeeRateValue);

I'm testing the above, see if it solves this issue. Probably a unit test with edge cases would be appropriate.

from walletwasabi.

MarnixCroes avatar MarnixCroes commented on July 18, 2024

I can repro it now with another transaction aswell

from walletwasabi.

nopara73 avatar nopara73 commented on July 18, 2024

cant repro

from walletwasabi.

yahiheb avatar yahiheb commented on July 18, 2024

This happened to me as well: #11224
5686d5d
I did cancel a tx but that is basically the same as a selfspend.

from walletwasabi.

lukasdll avatar lukasdll commented on July 18, 2024

After a lot more testing, I reached the conclusion that there is probably a rounding issue while calculating or subtracting the RBF fee to the output. That's why there is always 1 missing sat, and the transaction fails, but only when maths' want to make that rounding issue appear.

I found the place in the code where the magic happens and added quite a few logs.

It is in particular interesting:

Final amount in the selected own output after fee subtraction: 0.00005990

RBF Transaction Outputs: 0.00005991 to tb1qc4yczzve5mhj335qnrxggcqavvlvdmgp5j929u

Full logs below:

Transaction is successfully propagated: c8dbe5fd6a28ed792a6e68525106fb4790c345714880d497c9841c624c5b2028.
Starting to build a RBF transaction.
Best fee rate calculated: 1 Sat/B
Original fee: 0.00000159, Transaction size: 110 bytes
Original transaction fee rate: 1.445 Sat/B
Minimum RBF fee rate calculated: 2.445 Sat/B
Additional fee required for RBF: 0.00000110
Final RBF fee rate to use: 2.445 Sat/B
Selected own output for RBF: 0.00006100 to address tb1qc4yczzve5mhj335qnrxggcqavvlvdmgp5j929u
Final amount in the selected own output after fee subtraction: 0.00005990
Modifying the largest own output to cover the new fee.
Additional fee required for RBF: 0.00000110
Original transaction fee rate: 1.445 Sat/B
Final amount in the selected own output after fee subtraction: 0.00005990
Is the added fee sufficient for RBF? True
Final RBF transaction hash: 82d73ac7f5db214314c2c1c19877f63d9c1474ae11577de909052ad88fe67801
RBF Transaction Inputs: 5082130cb716ebc218fa313e4d2f278c9f8aa73d2595a3f57cc37e709763a7cf
RBF Transaction Outputs: 0.00005991 to tb1qc4yczzve5mhj335qnrxggcqavvlvdmgp5j929u
Total RBF Transaction Output Value: 5991
Final RBF Transaction Fee: 268
areForeignAmountsUnchanged: True
boostingTransaction.Transaction.GetWalletOutputs(_wallet.KeyManager).Any(): True
AreWePayingTheFee: True
Trying to broadcast transaction with random node (::ffff:127.0.0.1):82d73ac7f5db214314c2c1c19877f63d9c1474ae11577de909052ad88fe67801.
Successfully served transaction to node ([::ffff:127.0.0.1]:37150): 82d73ac7f5db214314c2c1c19877f63d9c1474ae11577de909052ad88fe67801.
Successfully served transaction to node ([::ffff:127.0.0.1]:37150): 82d73ac7f5db214314c2c1c19877f63d9c1474ae11577de909052ad88fe67801.
Disconnected node: ::ffff:127.0.0.1. Successfully broadcasted transaction: 82d73ac7f5db214314c2c1c19877f63d9c1474ae11577de909052ad88fe67801.
Round 38b34e58734f5666a82142e362b7f2f529bba5c54fee16630d4924c6e1339415 - Mining Fee Rate: 1 Sat/B, Network: TestNet, Max Suggested Amount: 43000.00000000, Min Amount Credential Value: 0.00005000
Random node could not broadcast transaction. Reason: Did not serve the transaction..
Broadcasting with backend...
ERROR  SpeedUpTransactionDialogViewModel.OnSpeedUpTransactionAsync (114)  System.Net.Http.HttpRequestException: Bad Request
insufficient fee, rejecting replacement 82d73ac7f5db214314c2c1c19877f63d9c1474ae11577de909052ad88fe67801, not enough additional fees to relay; 0.00000109 < 0.0000011:::01000000000101cfa76397707ec37cf5a395253da78a9f8c272f4d3e31fa18c2eb16b70c1382500100000000fdffffff01d417000000000000160014c549810999a6ef28c68098cc84601d633ec6ed010247304402200bbcae0911ec9aab6c0425c4681a0673937c201c96f0642b9373f0b442e6cc61022077204e7f8327553868b4bd4b78e912c4a272881f29f0c809e6a0dac43ffb8d1e012102439d930672257b4d348a4c5c056a9212a01827d0d9992ec1f12b7f366f8dbe762f382600
   at WalletWasabi.Tor.Http.Extensions.HttpResponseMessageExtensions.ThrowRequestExceptionFromContentAsync(HttpResponseMessage me, CancellationToken cancellationToken) in WalletWasabi\Tor\Http\Extensions\HttpResponseMessageExtensions.cs:line 122

https://mempool.space/testnet/tx/c8dbe5fd6a28ed792a6e68525106fb4790c345714880d497c9841c624c5b2028
RBF (failed): 82d73ac7f5db214314c2c1c19877f63d9c1474ae11577de909052ad88fe67801

I should be able to find a fix soon

from walletwasabi.

nopara73 avatar nopara73 commented on July 18, 2024

Is it possible to easily resolve it in NBitcoin? (Doesn't have to be quickly.) https://github.com/MetacoSA/NBitcoin/

from walletwasabi.

lukasdll avatar lukasdll commented on July 18, 2024

Indeed, it would be cleaner to solve it on NBitcoin. I will see what is possible to suggest on that end.

from walletwasabi.

nopara73 avatar nopara73 commented on July 18, 2024

Thank you, I appreciate it.

from walletwasabi.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.