IRC meeting summary for 2017-04-13
Notes / short topics
Ryanofsky’s multi-process changes where briefly discussed, it would separate QT from bitcoind. While separating the wallet from networking is more important, the GUI has more pressing problems as too much happens synchronous with the core in the GUI loop. The relevant PR #10102 isn’t finished but could use some review on the code that’s already there.
- Goals for 0.15
- High priority review
Bitcoin Core 0.14.1 includes bug-fixes and optimisations. Release candidate 1 (RC1) has been released on April 8th. RC2 has been released on April 17th (a few days after the meeting).
There are no reports of bugs in RC1. Cfields and BlueMatt still wants to get PR #10176 (gracefully handle NodeId wrapping) in 0.14.1, Gmaxwell and Wumpus prefer not to delay the release and don’t think it warrants another RC.
- decided post-meeting, but 0.14.1 got delayed and RC2 has been released.
CFields proposes a change to add a verifying script to the commit message, which can be verified by Travis. This verifying script can be used for simple things like a search and replace, which creates a lot diffs but is simple to script. If the script doesn’t transform the old commit into the new one exactly it would get rejected by Travis. This would make these simple changes with a lot of diffs easier to review.
Pull request #10193 is an example of how a scripted-diff commit should look.
Luke-jr thinks we shouldn’t trust Travis and it gives a false sense of review. Gmaxwell notes reviewers can test it after reviewing the script, the purpose of Travis is just for feedback that you’ve formatted it correctly and it runs on every computer, not as a review step.
Gmaxwell thinks it’s fine as long as we don’t expect commiters to be running it.
Wumpus thinks it’s a bit dangerous, as it would basically execute arbitrary scripts. Cfields notes this point was brought up by BlueMatt previously and thought it might be worth discussing constraining the script to ‘sed’, which is only substitution/replacement. Cfields also clarified the script only runs automatically on Travis, nowhere else.
Jtimon suggested that they should only run if there’s a scripted prefix in the commit title to make it much more obvious a script will be executed, as most reviewers review the code, not the commit message.
- Review PR #10189 (add a verifier for scriptable changes)
Goals for 0.15
Sdaftuar likes to know what everyones goals are for 0.15, so other people know what should have review priority.
Gmaxwell would like to see per-TXO dbcache and non-atomic flushing. Cfields wonders what the expected outcome for per-TXO is when downgrading from 0.15 to 0.14. Gmaxwell clarifies it can’t be downgraded and will need a reindex. It’s probably worth adding something into 0.14.2 which gracefully says the format has changed in an attempt do downgrade, which would be better than a generic corruption message.
Jonasschnelli’s goals are: HD auto-restore, QT feebumber, multiwallet and HD watch only wallet.
Sdaftuar would like to get the new fee estimation in place. Gmaxwell thinks we need a high level description of the algorithm which we can give to non-developers (academics) to review, it would also help general review as it’s easy to lose track of the overall design of the estimator. Morcos realizes it’s a giant pain to review for little perceived gain, but he does think it’s worth it. BlueMatt adds he does not think it’s little perceived gain, as one of the topics discussed in a wallet-dev meetup was how bad fee estimation across the ecosystem is, and Bitcoin Core is a big part of that. The new estimator greatly improves the fee-estimation during the weekends.
BlueMatt will be working on multithreaded net_processing (and wallet) with CValidationInterface generating callbacks into it.
Sipa would like to see per-TXO dbcache, removal of the memory peak at flushing and a better dbcache eviction policy.
High priority review
BlueMatt wants to add PR #10179 (Give CValidationInterface Support for calling notifications on the CScheduler Thread) as that and #10178 clear the way for his 0.15 goal of moving wallet callback into background threads.
Sipa wants to add PR #9792 (FastRandomContext improvements and switch to ChaCha20)
Morcos thinks #9942 (Refactor CBlockPolicyEstimator) can be merged, and would make the rest of the fee estimation changes smaller to review.
Jnewbery would like some review on #10044 (Run functional tests in ‘make check’)
|wumpus||Wladimir van der Laan|
|BlueMatt_||T-1000 Advanced Prototype (mimetic polyalloy)|
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.