|
| 1 | +--- |
| 2 | +title: IRC meeting summary for 2018-05-17 |
| 3 | +permalink: /en/meetings/2018/05/17/ |
| 4 | +name: 2018-05-17-meeting |
| 5 | +type: meetings |
| 6 | +layout: page |
| 7 | +lang: en |
| 8 | +version: 1 |
| 9 | +--- |
| 10 | +{% include _toc.html %} |
| 11 | +{% include _references.md %} |
| 12 | + |
| 13 | +- View this week's log on [BotBot.me](https://botbot.me/freenode/bitcoin-core-dev/msg/100174747/) or [MeetBot](http://www.erisian.com.au/meetbot/bitcoin-core-dev/2018/bitcoin-core-dev.2018-05-17-19.00.log.html) |
| 14 | +- [Meeting minutes by MeetBot](http://www.erisian.com.au/meetbot/bitcoin-core-dev/2018/bitcoin-core-dev.2018-05-17-19.00.html) |
| 15 | + |
| 16 | +--- |
| 17 | + |
| 18 | +Topics discussed during this weekly meeting included what pull requests |
| 19 | +the meeting participants most want to see reviewed, what issues need to |
| 20 | +be resolved before generating the first release candidate for version |
| 21 | +0.16.1, whether or not the project should leave GitHub, a request for |
| 22 | +review for a PR that paves the path towards separating node code from |
| 23 | +wallet code, and a potential new P2P protocol message to better handle |
| 24 | +relay of unverified blocks. |
| 25 | + |
| 26 | +## High priority for review |
| 27 | + |
| 28 | +**Background:** each meeting, Bitcoin Core developers discuss which Pull |
| 29 | +Requests (PRs) the meeting participants think most need review in the |
| 30 | +upcoming week. Some of these PRs are related to code that contributors |
| 31 | +especially want to see in the next release; others are PRs that are |
| 32 | +blocking further work or which require significant maintenance (rebasing) |
| 33 | +to keep in a pending state. Any capable reviewers are encouraged to |
| 34 | +visit the project's list of [current high-priority PRs][]. |
| 35 | + |
| 36 | +**Discussion ([log](http://www.erisian.com.au/meetbot/bitcoin-core-dev/2018/bitcoin-core-dev.2018-05-17-19.00.log.html#l-13)):** |
| 37 | +PRs specifically discussed included, |
| 38 | + |
| 39 | +- **[#12254][]:** BIP 158: Compact Block Filters for Light Clients |
| 40 | + |
| 41 | +- **[#12196][]:** Add scantxoutset RPC method |
| 42 | + |
| 43 | +- **[#13142][]:** Separate IsMine from solvability |
| 44 | + |
| 45 | +- **[#12979][]:** Make reusable base class for auxiliary indices |
| 46 | + |
| 47 | +In addition, Wladimir van der Laan expressed concerns that the list was |
| 48 | +getting quite long. |
| 49 | + |
| 50 | +## 0.16.1 |
| 51 | + |
| 52 | +**Background:** Bitcoin Core developers have begun preparing a new |
| 53 | +0.16.1 [maintenance release](/en/lifecycle/#maintenance-releases) with |
| 54 | +bugfixes and backports of important features. |
| 55 | + |
| 56 | +**Discussion |
| 57 | +([log](http://www.erisian.com.au/meetbot/bitcoin-core-dev/2018/bitcoin-core-dev.2018-05-17-19.00.log.html#l-65)):** |
| 58 | +Wladimir van der Laan proposed the topic and provided a brief survey of |
| 59 | +the work that still needs to be done: |
| 60 | + |
| 61 | +1. **[0.16] Further Backports ([#13253][]).** Needs more review. |
| 62 | + |
| 63 | +2. **0.16.0 bitcoin-qt: "Assertion `copyFrom failed" during launch |
| 64 | + ([#13110][]).** Van der Laan says he "proposed a fix for [this] and |
| 65 | + it apparently worked." |
| 66 | + |
| 67 | +3. **Assertion failure during rescan ([#12646][]).** Jonas Schnelli |
| 68 | + suggested bumping, and Van der Laan agreed. It was retargeted to |
| 69 | + 0.16.2. |
| 70 | + |
| 71 | +4. **0.16 Shutdown assertion ([#12337][]).** Schnelli is investigating |
| 72 | + this. |
| 73 | + |
| 74 | +**Conclusion:** "We just need to finish backports and tag for |
| 75 | +0.16.1RC1," said Matt Corallo. |
| 76 | + |
| 77 | +## Trashing GitHub |
| 78 | + |
| 79 | +**Background:** for more than 6 weeks now, the webpages for |
| 80 | +highly-reviewed Bitcoin Core PRs on GitHub have frequently failed to |
| 81 | +load, with reviewers seeing an illustration of an angry unicorn instead. |
| 82 | +The issue has been reported several times to GitHub support by different |
| 83 | +people but has not yet been resolved. |
| 84 | + |
| 85 | +**Discussion |
| 86 | +([log](http://www.erisian.com.au/meetbot/bitcoin-core-dev/2018/bitcoin-core-dev.2018-05-17-19.00.log.html#l-97)):** |
| 87 | +Matt Corallo requested the topic and introduced it: "It hasn't been |
| 88 | +working [...] and I'd kinda like to have something self-hosted with |
| 89 | +better review tools anyway, which I know a lot of people wanted." |
| 90 | + |
| 91 | +Pieter Wuille and Wladimir van der Laan suggested that GitLab was an |
| 92 | +alternative, which Corallo accepted but noted, "though GitLab seems to |
| 93 | +have no better review tools than GitHub." |
| 94 | + |
| 95 | +Suhas Daftuar was concerned that, "it seems to me like it's way harder |
| 96 | +to get it right hosting ourselves." Van der Laan had the same concern, |
| 97 | +"Who is going to babysit this, monitor it, and apply security patches, |
| 98 | +etc...?" |
| 99 | + |
| 100 | +Cory Fields added, "General NACK: self-hosting issues aside, GitHub's |
| 101 | +network effect is too strong [in my opinion]. I can't be the only one |
| 102 | +who gets irrationally frustrated when the code I want to mess with is on |
| 103 | +BitBucket." Van der Laan agreed, "Yes, only [large] players like |
| 104 | +FreeDesktop can really afford to host on separate infrastructure; for |
| 105 | +smaller projects the lack of network effect (and having to register |
| 106 | +separately) is bad." John Newbery also agreed. |
| 107 | + |
| 108 | +Talking about features he'd like to see in a GitHub replacement, Corallo |
| 109 | +wished there was a command-line way to "verify, e.g., PGP signatures on |
| 110 | +comments." That way if anyone compromised the repository web service to |
| 111 | +forge an ACK on a PR, it could be detected before merge. |
| 112 | + |
| 113 | +Jim Posen and Steve Lee offered to help get more information about |
| 114 | +the issue from GitHub. The Bitcoin Core project is not the only one |
| 115 | +suffering from the issue, with Corallo saying "Some other projects were |
| 116 | +posting responses they got where [GitHub support was] saying, 'we don't |
| 117 | +actually know what change we made that triggered these issues, hold |
| 118 | +on.' But that was three weeks ago." |
| 119 | + |
| 120 | +**Conclusion:** Corallo decided there was too much adversity to the idea |
| 121 | +at the moment, so "I'm not gonna spend time looking into it." Van der |
| 122 | +Laan concluded, "I don't think there's realistically any chance of |
| 123 | +anything replacing GitHub until someone sets up a feasible alternative |
| 124 | +and shows us that it is better." |
| 125 | + |
| 126 | +**Post-meeting:** about a day after the meeting, Jonas Schnelli received |
| 127 | +a |
| 128 | +[message](https://0bin.net/paste/ViHtKCgPIfW0TMYt#j30mFske0y1EVoVRZCsQqMoYCpoPc3axAV29jkKkznB) |
| 129 | +from Ben Balter, a product manager at GitHub, saying that GitHub has |
| 130 | +"identified the root cause, and are working on a fix." |
| 131 | + |
| 132 | +## Separate wallet from node |
| 133 | + |
| 134 | +**Background:** Bitcoin Core's full node implementation, wallet, and |
| 135 | +Graphical User Interface (GUI) all currently run as a single process |
| 136 | +(although the wallet and the GUI can be disabled). This means, for |
| 137 | +example, that if you close the GUI, you also stop the node. It has been |
| 138 | +a long-term goal of several contributors to split these different parts |
| 139 | +into separate processes so that they can be operated independently of |
| 140 | +one another. |
| 141 | + |
| 142 | +**Discussion |
| 143 | +([log](http://www.erisian.com.au/meetbot/bitcoin-core-dev/2018/bitcoin-core-dev.2018-05-17-19.00.log.html#l-234)):** |
| 144 | +John Newbery requested the topic and introduced it: "[#10973][] is a |
| 145 | +big PR, but I think it's very worthwhile, [...] but it requires |
| 146 | +continual rebase. [...] I think it'd be great to make some progress on |
| 147 | +this one." |
| 148 | + |
| 149 | +Wladimir van der Laan was concerned that the high-priority review queue |
| 150 | +is too large: "Oh, no, not more high priority for review. Is it |
| 151 | +blocking anything? Is it important for 0.17? Process separation is not |
| 152 | +something we'll have for 0.17 anyway." |
| 153 | + |
| 154 | +The author of the PR, Russell Yanofsky, offered to split the first six |
| 155 | +commits off to a separate PR so that they can be reviewed independently |
| 156 | +of the later commits, reducing the size of the PR and hopefully making |
| 157 | +it easier to review. |
| 158 | + |
| 159 | +**Conclusion:** With Yanofsky offering to split the PR and a few |
| 160 | +contributors offering to review it in the upcoming week, Newbery closed |
| 161 | +the topic. |
| 162 | + |
| 163 | +## Unverified-block-message |
| 164 | + |
| 165 | +**Background:** [BIP152][] compact block relay introduced a |
| 166 | +high-bandwidth mode where a node can send information to its peers about |
| 167 | +a new block before the node has finished validating that block. If the |
| 168 | +node does finish validating the block and finds that the block is invalid, |
| 169 | +but the peers request the whole block anyway, there's currently no way |
| 170 | +for the node to tell its peers that it doesn't have a valid block to send |
| 171 | +them. Currently, in this case, the peers will eventually disconnect |
| 172 | +from the node for failing to send the requested block. |
| 173 | + |
| 174 | +**Discussion |
| 175 | +([log](http://www.erisian.com.au/meetbot/bitcoin-core-dev/2018/bitcoin-core-dev.2018-05-17-19.00.log.html#l-268)):** |
| 176 | +Matt Corallo requested the topic and introduced it by describing two |
| 177 | +potential solutions to the problem: |
| 178 | + |
| 179 | +1. The node tells requesting peers that it refuses to relay the block. |
| 180 | + |
| 181 | +2. The node gives the peers the requested block, proving it has valid |
| 182 | + block headers (as required by BIP152), but also tags it as |
| 183 | + potentially invalid. |
| 184 | + |
| 185 | +Pieter Wuille suggested that the existing [`notfound`][p2p notfound] |
| 186 | +message could possibly be reused as part of implementing the first |
| 187 | +proposed solution. |
| 188 | + |
| 189 | +Suhas Daftuar argued against reusing the `notfound` message in favor of |
| 190 | +the second proposed solution: "I think notfounds are worse because of |
| 191 | +the case where the block might not have been validated either way." |
| 192 | +Wladimir van der Laan agreed that a new message should be used "if |
| 193 | +there's no specific reason to re-use `notfound`, a new message is much |
| 194 | +better." |
| 195 | + |
| 196 | +**Conclusion:** Corallo was still considering the options, but thought |
| 197 | +it was "good to ask. Obviously [the actual solution] requires a BIP and |
| 198 | +whatever else." |
| 199 | + |
| 200 | +## Other topics |
| 201 | + |
| 202 | +With just minutes remaining in the meeting, Matt Corallo proposed |
| 203 | +a topic titled "Queue drain lock assertions to avoid deadlocks," but |
| 204 | +there wasn't enough time to discuss the topic and Corallo said, "I |
| 205 | +realize now I should just open a PR and people will see it, [as] it's |
| 206 | +kinda knotty to describe." |
| 207 | + |
| 208 | +## Comic relief |
| 209 | + |
| 210 | +{% highlight text %} |
| 211 | +<BlueMatt> trashing github |
| 212 | +<BlueMatt> or we could switch to gitlab |
| 213 | + <sipa> let's move back to sourceforge |
| 214 | +{% endhighlight %} |
| 215 | + |
| 216 | +{% highlight text %} |
| 217 | +<sdaftuar> i think we could just add a new BLOKC response type |
| 218 | + BLOCK_COULDBEBAD |
| 219 | + <sipa> 0xDEADB10C |
| 220 | + <wumpus> hehe |
| 221 | +{% endhighlight %} |
| 222 | + |
| 223 | + |
| 224 | +## Participants |
| 225 | + |
| 226 | +| IRC nick | Name/Nym | |
| 227 | +|-----------------|---------------------------| |
| 228 | +| BlueMatt | [Matt Corallo][] | |
| 229 | +| wumpus | [Wladimir van der Laan][] | |
| 230 | +| sipa | [Pieter Wuille][] | |
| 231 | +| jonasschnelli | [Jonas Schnelli][] | |
| 232 | +| jnewbery | [John Newbery][] | |
| 233 | +| jtimon | [Jorge Timón][] | |
| 234 | +| jimpo | [Jim Posen][] | |
| 235 | +| sdaftuar | [Suhas Daftuar][] | |
| 236 | +| cfields | [Cory Fields][] | |
| 237 | +| MarcoFalke | [Marco Falke][] | |
| 238 | +| jamesob | [James O'Beirne][] | |
| 239 | +| promag | [Joao Barbosa][] | |
| 240 | +| moneyball | [Steve Lee][] | |
| 241 | +| ryanofsky | [Russell Yanofsky][] | |
| 242 | +| kanzure | [Bryan Bishop][] | |
| 243 | + |
| 244 | +## Disclaimer |
| 245 | + |
| 246 | +This summary was compiled without input from any of the participants in |
| 247 | +the discussion, so any errors are the fault of the summary author and |
| 248 | +not the discussion participants. In particular, quotes taken from the |
| 249 | +discussion had their capitalization, punctuation, and spelling modified |
| 250 | +to produce consistent sentences. Bracketed words and fragments, as well |
| 251 | +as background narratives and exposition, were added by the author of |
| 252 | +this summary and may have accidentally changed the meaning of some |
| 253 | +sentences. If you believe any quote was taken out of context, please |
| 254 | +[open an issue](https://github.com/bitcoin-core/bitcoincore.org/issues/new) |
| 255 | +and we will correct the mistake. |
| 256 | + |
| 257 | +[#10973]: https://github.com/bitcoin/bitcoin/issues/10973 |
| 258 | +[#12196]: https://github.com/bitcoin/bitcoin/issues/12196 |
| 259 | +[#12254]: https://github.com/bitcoin/bitcoin/issues/12254 |
| 260 | +[#12337]: https://github.com/bitcoin/bitcoin/issues/12337 |
| 261 | +[#12646]: https://github.com/bitcoin/bitcoin/issues/12646 |
| 262 | +[#12979]: https://github.com/bitcoin/bitcoin/issues/12979 |
| 263 | +[#13110]: https://github.com/bitcoin/bitcoin/issues/13110 |
| 264 | +[#13142]: https://github.com/bitcoin/bitcoin/issues/13142 |
| 265 | +[#13253]: https://github.com/bitcoin/bitcoin/issues/13253 |
| 266 | +[p2p notfound]: https://bitcoin.org/en/developer-reference#notfound |
| 267 | +[current high-priority PRs]: https://github.com/bitcoin/bitcoin/projects/8 |
0 commit comments