Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

InvalidTxExcpetion - Fee too low #2111

Closed
lontivero opened this issue Aug 11, 2019 · 6 comments · Fixed by #2047 or #2116
Closed

InvalidTxExcpetion - Fee too low #2111

lontivero opened this issue Aug 11, 2019 · 6 comments · Fixed by #2047 or #2116
Labels

Comments

@lontivero
Copy link
Collaborator

It seems Wasabi in some situations calculates a transaction fee that is below the MinFeePolicy calculated by NBitcoin.

@NicolasDorier suggests Wasabi could use TransactionBuilder::SendEstimatedFee method to calculate the fee instead of using its own fee calculation algorithm.

@NicolasDorier
Copy link
Contributor

To be precise Wasabi may generate transactions below the minrelayfee because of rounding error. Using the SendEstimatedFee would fix it.

@benthecarman
Copy link
Contributor

benthecarman commented Aug 12, 2019

In the build transaction function we just do

Transaction tx = builder
		.SetChange(changeScriptPubKey)
		.SendFees(fee)
		.BuildTransaction(sign);

if we change it to

Transaction tx = builder
		.SetChange(changeScriptPubKey)
		.SendEstimatedFees(feePerBytes)
		.BuildTransaction(sign);

It looks like it'll only affect the case where subtractFeeFromAmountIndex is set and if it is a rounding error then it'll only be a 1 satoshi amount change.

Since it is only called from the send tab and we do not set subtractFeeFromAmountIndex it should be fine to just switch from SendFees() to SendEstimatedFees() with no functional difference.

@nopara73
Copy link
Contributor

The 2s/b sanity limit was removed with the manual fee estimation PR and it got merged, even though the regtests brought out this error. In fact I fixed it a while ago in this PR it just didn't get merged yet: https://github.com/zkSNACKs/WalletWasabi/pull/2047/files#diff-0a80ca357eb532ff3ed358fb4c8caae1R1156-R1161

@nopara73
Copy link
Contributor

Merged.

@NicolasDorier
Copy link
Contributor

@nopara73 please use SendEstimatedFees of NBitcoin. I spend lot's of time to get it right, manually calculating it is very hard.

@benthecarman NBitcoin has a SubractFee() which allow to substract the fees from the destination already.

@nopara73
Copy link
Contributor

@NicolasDorier You trust your code more and I trust my code more 😄 Although I'm pretty sure the regtests would catch if there are issues with your code, so I'm not against a refactoring.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants