Warn when transaction fee might be higher due to block chain out of sync #1858
Comments
|
This needs some re-work yes, we have quite some open related issues to this on Github. |
|
I suppose sending transactions with a non-up-to-date block chain should be discouraged? Edit: in principle, only when inputs have been selected that were received recently, so it influences the fee computation. But that'd be lots more complex both for the user and implementation... |
|
I like the idea of simply not allowing transactions when we are not up-to-date chain wise. Still leaves the rest of the fee thing open, but addresses this issue. |
|
If old inputs are being used but they are all very small, could that also trigger the fee? In which case a newer larger input might be able to remove the need for a fee. So just checking for inputs which were received recently might not solve it? I think having a warning would be best if the chain is not in sync and the client currently thinks a fee is needed. That way the user has the option. |
|
We don't currently reliably know when we're not in sync, unless it's so far back that it's before the highest checkpoint. The median measure is absolutely unacceptable for this kind of usage: doing that would allow a greifer to spin up a bunch of height lying nodes and block all (with fee?) transactions. |
|
I also don't agree with blocking transactions, the user should always be able to do that. But adding a warning that the user could be paying more fee that needed would be fine, IMO. The same check could be used that is now used for the "out of sync" warnings. The median is not even used for that, just the simple measure that the last generated block was within N minutes. |
|
There are warning icons in the GUI now when the chain is not up to date. |
…FeeTX indexes d7a1c99 [Cleanup] Extra call to CBudgetMan::SetBestHeight in reconsiderblock (random-zebra) 5d0731e [Cleanup] Remove redundant Clear in CBudgetManager constructor (random-zebra) 48b723e [Budget] Remove proposal/budgets if FeeTx block is disconnected (random-zebra) 9c6173c [Budget] Introduce mapFeeTxToProposal and mapFeeTxToBudget (random-zebra) Pull request description: Based on top of: - [x] bitcoin#1851 Starting with `[Budget] Introduce mapFeeTxToProposal and mapFeeTxToBudget` (0eb9e45) Since bitcoin#1845 we no longer check repeatedly the active chain, when updating a budget object, we just save the height of the block including the collateral tx (ref bitcoin#1845), when we first add the proposal/budget to the map. Therefore we need to handle the case where said block is disconnected from the chain. In order to do so efficiently, this PR adds two indexes (`mapFeeTxToBudget`/`mapFeeTxToProposal`), mapping collateral txids to the relative budget objects (while such objects are stored in the map). A simple function `CBudgetManager::RemoveByFeeTxId()` can then be called from `DisconnectBlock` when undoing transactions, in order to remove the now-conflicted budget objects (without performing any expensive search). ACKs for top commit: furszy: nice, code review ACK d7a1c99. Fuzzbawls: ACK d7a1c99 Tree-SHA512: 9dd8a671032f79105219b7a0e3e0514b991ff92e55834f5adad4964fb9b5c1842bbb0c0995c9b504ca79c5c86115061f4a93e9b4560a63c2beaf66aa5915c2f5
…r finalized budgets 5c0d2aa [Refactor] Fix styling (random-zebra) bcc7d7c [Cleanup] Remove not needed comparator (random-zebra) 8cf2ef0 [Trivial] Pass references to vote constructors (random-zebra) 56f176a [Trivial] Pass manager object to DumpBudgets (random-zebra) 7a5c02a [Refactor] Don't re-hash proposals in CFinalizedBudget::CheckProposals (random-zebra) b454d60 [Refactor] GetBudget() return proposal copies instead of pointers (random-zebra) b6af31c [Cleanup] Don't lock cs_budgets before CBudgetManager::SubmitVote (random-zebra) cbc6583 [Refactor] Check finalized budget inside CBudgetManager::SubmitVote (random-zebra) 9eceb21 [Refactor] Move finalized budget vote submission to budget-manager (random-zebra) 001f15d [Trivial] Rename global budget object to g_budgetman (random-zebra) Pull request description: Based on top of - [x] bitcoin#1858 Starting with `[Trivial] Rename global budget object to g_budgetman` (64f0840) This refactors `CFinalizedBudget::SubmitVote`, moving it to the budget manager (`VoteOnFinalizedBudgets`), and removing the nested locking issues. It also does some trivial cleanup: - rename the global budget manager `g_budgetman` - pass const references to `CBudgetVote`/`CFinalizedBudget` constructors - pass the manager externally to `DumpBudgets` (instead of using the global one) ACKs for top commit: furszy: utACK 5c0d2aa after rebase for 1916 and merging.. Tree-SHA512: 0d12ca38f177e6fd5b1013f118375ca37a498593076cdc1877f5fb0e9329dd8e7292beee866600bc98b002476b3d9d44dac1d2cb2adbaa7276b52d97c5702119
I was going to send a transaction but it was too large and my client (0.7.0) wanted me to pay a fee. However, I was not fully synchronized with the blockchain yet. The client knew this. I waited a few more blocks and sent the transaction without having to pay the fee.
The message asking me to pay a fee didn't mention the blockchain not being synchronized. Users who are unaware of how fees are calculated could have paid an unnecessary fee to the network had they accepted prior to the client knowing that there was a way to send coins w/o the fee. Perhaps the error message could be modified to reflect this?
Laanwj: clarified issue title
The text was updated successfully, but these errors were encountered: