IRC meeting summary for 2016-12-08
Notes / short topics
- Github now supports listing reviewers for your PR
- Segregated witness support for the getblocktemplate implementation needs some urgent review as it’s required for some downstream miners for segwit
- Make RelayWalletTransaction attempt to AcceptToMemoryPool
- mempool expiry time increase
Make RelayWalletTransaction attempt to AcceptToMemoryPool
Pull request #9290 (Make RelayWalletTransaction attempt to AcceptToMemoryPool) fixes an issue where a wallet transaction which failed to relay previously because it couldn’t make it into the mempool will not try again until restart, even though mempool conditions may have changed. Together with pull request #9262 (Prefer coins that have fewer ancestors, sanity check txn before ATMP) they fix cases where normal use of the wallet for some users can cause inexplicable failures due to creating long transaction chains in the mempool. These long transaction chains will look like the send failed, but it will still go into the wallet and still be broadcast later potentially (After a restart). Users lose access to their funds and may falsely believe a wallet is empty. Resulting in a possible double pay.
The concern expressed in the description of the pull request was from morcos, however he was convinced by sdaftuar and agrees with the PR. He still has some doubts about backporting it and definitely for #9262. He worries that, as these are fairly large changes in behavior, they won’t get enough testing to be sure they don’t raise new issues.
This behavior has been like this for several versions, however this has recently become a bigger issue with the occasional mempool backlogs.
Gmaxwell would like to see either #9262 or #9290 being backported, but if only one prefers #9290. Morcos also feels easier on #9290 as it’s pretty simple.
Morcos and Wumpus would like to see focus on a good solution for 0.14 instead or rushing for 0.13.2. RC1 for 0.13.2 should probably happen in December to avoid overlap with 0.14.
Sipa wonders whether there’s a patch that deals with AcceptToMemoryPool (ATMP) failing in createTransaction. #9262 would make it much less likely that you will get to ATMP fail. #9262 makes it much less likely you’ll get ATMP fail, but sipa would be much more comfortable with something that deals correctly with an occasional failure rather than trying our best to avoid failures. This way you’d know your transaction was not broadcast immediately. Gmaxwell argues we never really know that since we have no monitoring to tell if the broadcasts were successful.
Sdaftuar proposes a simple backport which would return the transaction ID of the failed-to-ATMP transaction back to the RPC caller once it’s been added to the wallet. Sipa worked on a pull request during the meeting.
Luke-jr notes the issue could also be addressed by setting the default of -spendzeroconfchange to 0, however that would be a disruptive change and should only be considered when it’s a really critical problem.
- Review for master and backports #9290 (Make RelayWalletTransaction attempt to AcceptToMemoryPool), #9302 (Return txid even if ATMP fails for new transaction) and optional for backport #9262 (Prefer coins that have fewer ancestors, sanity check txn before ATMP)
mempool expiry time increase
Transactions that currently sit in the memory pool for longer than 3 days are removed from the mempool. Morcos proposes to increase this expiry time to 2 weeks.
Morcos argues if we want to fully utilize weekly cycles in transaction volume, we need to have transactions that sit around for a week or longer to measure how long it takes them to get confirmed.
Gmaxwell notes the expiration removes high fee transactions that got softforked out but is taking up your mempool. However a 3-day expiry time still messes up the fee estimation anyway.
Sdaftuar notes another advantage of 3 days versus a week is being able to double spend a too low fee transaction, however after introducing fee bumping this problem largely goes away. Gmaxwell argues replacement of non-replaceable transactions works now even after a day, due to restarts and full-rbf miners.
- Make a PR to increase the expiry time and get more thoughts and issues.
|wumpus||Wladimir van der Laan|
This summary was compiled without input from any of the participants in the discussion, so any errors are the fault of the summary author and not the discussion participants.