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.

Main topics

0.14.1

scripted-diffs

Goals for 0.15

High priority review

0.14.1

background

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.

meeting conclusion

decided post-meeting, but 0.14.1 got delayed and RC2 has been released.

scripted-diffs

background

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.

meeting conclusion

Review PR #10189 (add a verifier for scriptable changes)

Goals for 0.15

background

Bitcoin Core 0.15.0 is scheduled to be released around September 2017. Pull requests aimed for 0.15 are tagged with a 0.15.0 tag.

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’)

Comic relief

BlueMatt because its free, we're already doing 0.14.1 and delaying 1 week isnt gonna kill us BlueMatt_ But delaying 1 week isn't too bad BlueMatt wait, who is BlueMatt_ ? wumpus confused BlueMatt_ confused BlueMatt has no idea who BlueMatt_ is BlueMatt_ has no idea who BlueMatt is kanzure different timeline, carry on. luke-jr whois says it's Matt Corallo BlueMatt not me gmaxwell wumpus: shoot the T1000 (BlueMatt_) and lets move on. sipa BlueMatt_: this statement is false spudowiar You could create format like '*.cpp *.h | s/boost::filesystem/fs/g' sipa spudowiar: little bobby tables will haunt you sipa I want pertxoutcache, remove memory peak at flushing, better dbcache eviction policy, ... sipa oh, and segwit activated? pretty please? BlueMatt sipa: lol cfields_ sipa: let's activate segwit after the meeting. We only have 20 min left :p gmaxwell cfields_: ack wumpus #action activate segwit gmaxwell jinx

Participants

Disclaimer

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.