IRC meeting summary for 2018-05-17

• View this week’s log on BotBot.me or MeetBot
• Meeting minutes by MeetBot

Topics discussed during this weekly meeting included what pull requests the meeting participants most want to see reviewed, what issues need to be resolved before generating the first release candidate for version 0.16.1, whether or not the project should leave GitHub, a request for review for a PR that paves the path towards separating node code from wallet code, and a potential new P2P protocol message to better handle relay of unverified blocks.

High priority for review

Background: each meeting, Bitcoin Core developers discuss which Pull Requests (PRs) the meeting participants think most need review in the upcoming week. Some of these PRs are related to code that contributors especially want to see in the next release; others are PRs that are blocking further work or which require significant maintenance (rebasing) to keep in a pending state. Any capable reviewers are encouraged to visit the project’s list of current high-priority PRs.

Discussion (log): PRs specifically discussed included,

• #12254: BIP 158: Compact Block Filters for Light Clients

• #12196: Add scantxoutset RPC method

• #13142: Separate IsMine from solvability

• #12979: Make reusable base class for auxiliary indices

In addition, Wladimir van der Laan expressed concerns that the list was getting quite long.

0.16.1

Background: Bitcoin Core developers have begun preparing a new 0.16.1 maintenance release with bugfixes and backports of important features.

Discussion (log): Wladimir van der Laan proposed the topic and provided a brief survey of the work that still needs to be done:

1. [0.16] Further Backports (#13253). Needs more review.

2. 0.16.0 bitcoin-qt: “Assertion `copyFrom failed” during launch (#13110). Van der Laan says he “proposed a fix for [this] and it apparently worked.”

3. Assertion failure during rescan (#12646). Jonas Schnelli suggested bumping, and Van der Laan agreed. It was retargeted to 0.16.2.

4. 0.16 Shutdown assertion (#12337). Schnelli is investigating this.

Conclusion: “We just need to finish backports and tag for 0.16.1RC1,” said Matt Corallo.

Trashing GitHub

Background: for more than 6 weeks now, the webpages for highly-reviewed Bitcoin Core PRs on GitHub have frequently failed to load, with reviewers seeing an illustration of an angry unicorn instead. The issue has been reported several times to GitHub support by different people but has not yet been resolved.

Discussion (log): Matt Corallo requested the topic and introduced it: “It hasn’t been working […] and I’d kinda like to have something self-hosted with better review tools anyway, which I know a lot of people wanted.”

Pieter Wuille and Wladimir van der Laan suggested that GitLab was an alternative, which Corallo accepted but noted, “though GitLab seems to have no better review tools than GitHub.”

Suhas Daftuar was concerned that, “it seems to me like it’s way harder to get it right hosting ourselves.” Van der Laan had the same concern, “Who is going to babysit this, monitor it, and apply security patches, etc…?”

Cory Fields added, “General NACK: self-hosting issues aside, GitHub’s network effect is too strong [in my opinion]. I can’t be the only one who gets irrationally frustrated when the code I want to mess with is on BitBucket.” Van der Laan agreed, “Yes, only [large] players like FreeDesktop can really afford to host on separate infrastructure; for smaller projects the lack of network effect (and having to register separately) is bad.” John Newbery also agreed.

Talking about features he’d like to see in a GitHub replacement, Corallo wished there was a command-line way to “verify, e.g., PGP signatures on comments.” That way if anyone compromised the repository web service to forge an ACK on a PR, it could be detected before merge.

Jim Posen and Steve Lee offered to help get more information about the issue from GitHub. The Bitcoin Core project is not the only one suffering from the issue, with Corallo saying “Some other projects were posting responses they got where [GitHub support was] saying, ‘we don’t actually know what change we made that triggered these issues, hold on.’ But that was three weeks ago.”

Conclusion: Corallo decided there was too much adversity to the idea at the moment, so “I’m not gonna spend time looking into it.” Van der Laan concluded, “I don’t think there’s realistically any chance of anything replacing GitHub until someone sets up a feasible alternative and shows us that it is better.”

Post-meeting: about a day after the meeting, Jonas Schnelli received a message from Ben Balter, a product manager at GitHub, saying that GitHub has “identified the root cause, and are working on a fix.”

Separate wallet from node

Background: Bitcoin Core’s full node implementation, wallet, and Graphical User Interface (GUI) all currently run as a single process (although the wallet and the GUI can be disabled). This means, for example, that if you close the GUI, you also stop the node. It has been a long-term goal of several contributors to split these different parts into separate processes so that they can be operated independently of one another.

Discussion (log): John Newbery requested the topic and introduced it: “#10973 is a big PR, but I think it’s very worthwhile, […] but it requires continual rebase. […] I think it’d be great to make some progress on this one.”

Wladimir van der Laan was concerned that the high-priority review queue is too large: “Oh, no, not more high priority for review. Is it blocking anything? Is it important for 0.17? Process separation is not something we’ll have for 0.17 anyway.”

The author of the PR, Russell Yanofsky, offered to split the first six commits off to a separate PR so that they can be reviewed independently of the later commits, reducing the size of the PR and hopefully making it easier to review.

Conclusion: With Yanofsky offering to split the PR and a few contributors offering to review it in the upcoming week, Newbery closed the topic.

Unverified-block-message

Background: BIP152 compact block relay introduced a high-bandwidth mode where a node can send information to its peers about a new block before the node has finished validating that block. If the node does finish validating the block and finds that the block is invalid, but the peers request the whole block anyway, there’s currently no way for the node to tell its peers that it doesn’t have a valid block to send them. Currently, in this case, the peers will eventually disconnect from the node for failing to send the requested block.

Discussion (log): Matt Corallo requested the topic and introduced it by describing two potential solutions to the problem:

1. The node tells requesting peers that it refuses to relay the block.

2. The node gives the peers the requested block, proving it has valid block headers (as required by BIP152), but also tags it as potentially invalid.

Pieter Wuille suggested that the existing notfound message could possibly be reused as part of implementing the first proposed solution.

Suhas Daftuar argued against reusing the notfound message in favor of the second proposed solution: “I think notfounds are worse because of the case where the block might not have been validated either way.” Wladimir van der Laan agreed that a new message should be used “if there’s no specific reason to re-use notfound, a new message is much better.”

Conclusion: Corallo was still considering the options, but thought it was “good to ask. Obviously [the actual solution] requires a BIP and whatever else.”

Other topics

With just minutes remaining in the meeting, Matt Corallo proposed a topic titled “Queue drain lock assertions to avoid deadlocks,” but there wasn’t enough time to discuss the topic and Corallo said, “I realize now I should just open a PR and people will see it, [as] it’s kinda knotty to describe.”

Comic relief

<BlueMatt> trashing github
<BlueMatt> or we could switch to gitlab
    <sipa> let's move back to sourceforge

<sdaftuar> i think we could just add a new BLOKC response type
           BLOCK_COULDBEBAD
    <sipa> 0xDEADB10C
  <wumpus> hehe

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. In particular, quotes taken from the discussion had their capitalization, punctuation, and spelling modified to produce consistent sentences. Bracketed words and fragments, as well as background narratives and exposition, were added by the author of this summary and may have accidentally changed the meaning of some sentences. If you believe any quote was taken out of context, please open an issue and we will correct the mistake.