Skip to content

Commit bd9e2dc

Browse files
hardingharding
committed
Meetings: add 2018-05-03
1 parent 828b5b2 commit bd9e2dc

File tree

2 files changed

+300
-0
lines changed

2 files changed

+300
-0
lines changed

‎_includes/_references.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
[Gregory Sanders]: https://github.com/instagibbs
1717
[James O'Beirne]: https://github.com/jamesob
1818
[Jeremy Rubin]: https://github.com/JeremyRubin
19+
[Jesse Cohen]: http://github.com/skeees
1920
[Jim Posen]: https://github.com/jimpo
2021
[Joao Barbosa]: https://github.com/promag
2122
[John Newbery]: https://github.com/jnewbery

‎_posts/en/meetings/2018-05-03-meeting.md

Lines changed: 299 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,299 @@
1+
---
2+
title: IRC meeting summary for 2018-05-03
3+
permalink: /en/meetings/2018/05/03/
4+
name: 2018-05-03-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 logs on [BotBot.me](https://botbot.me/freenode/bitcoin-core-dev/msg/99670696/) or [MeetBot](http://www.erisian.com.au/meetbot/bitcoin-core-dev/2018/bitcoin-core-dev.2018-05-03-19.00.log.html)
14+
- [Meeting minutes by MeetBot](http://www.erisian.com.au/meetbot/bitcoin-core-dev/2018/bitcoin-core-dev.2018-05-03-19.00.html)
15+
16+
---
17+
18+
Topics discussed during this weekly meeting included what pull requests
19+
members of the project would like reviewers to focus on during the
20+
upcoming week, whether debug logging should be moved to a separate
21+
thread so that it doesn't slow down certain use cases, whether to produce a
22+
0.16.1 minor release containing some bugfixes and standard transaction
23+
policy changes, and how to allow messages received from other peers on
24+
the network to be processed in parallel.
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
35+
PRs][].
36+
37+
**Discussion
38+
([log](http://www.erisian.com.au/meetbot/bitcoin-core-dev/2018/bitcoin-core-dev.2018-05-03-19.00.log.html#l-24)):**
39+
Specific PRs discussed during the meeting included,
40+
41+
1. **BIP 158: Compact Block Filters for Light Clients ([#12254][]),**
42+
nominated for inclusion on the list by Jim Posen. This PR allows
43+
Bitcoin Core to generate compact indices for some of the information
44+
contained within a block. These indices can then be shared with
45+
lightweight clients to allow them to determine whether or not the
46+
block contains information relevant to the client's wallet, at which
47+
point the client can request to download the whole block (possibly
48+
from a different peer).
49+
50+
2. **[wallet] `loadwallet` RPC - load wallet at runtime ([#10740][]),**
51+
nominated for inclusion on the list by John Newbery. This is one of
52+
a set of PRs that (if accepted) will allow Bitcoin Core to create,
53+
load, and unload wallets at runtime in the context of the multiwallet
54+
mode added in Bitcoin Core 0.15.0.
55+
56+
3. **UI: Support wallets loaded dynamically ([#13097][]),** nominated for
57+
inclusion on the list by Joao Barbosa. Built on top of the
58+
previously-mentioned PR #10740, this provides support in the Bitcoin
59+
Core Graphical User Interface (GUI) for dynamic loading of wallets.
60+
61+
The issue of some GitHub pages not loading ("unicorns") was briefly
62+
mentioned in this meeting again for at least the fourth week running.
63+
64+
## Delete 0.8, 0.9, and 0.10 git branches
65+
66+
**Background:** new development of Bitcoin Core generally occurs on the
67+
`master` branch of the Git repository. To create a stable release, the
68+
master branch is git-forked into a stable branch with the name of the
69+
intended release, e.g. for version 0.8 the branch is `0.8`. The code on
70+
this branch is tested, matured, and released---and subsequent bug fixes
71+
for minor version releases (e.g. 0.8.1) are also made on this branch.
72+
73+
**Discussion
74+
([log](http://www.erisian.com.au/meetbot/bitcoin-core-dev/2018/bitcoin-core-dev.2018-05-03-19.00.log.html#l-61)):**
75+
Marco Falke proposed the topic shortly before the meeting and introduced
76+
it saying, "Those last tagged versions on those branches are EOL [End Of
77+
Life] for more than a year now. The tags can be kept for archival
78+
reasons, but the branches are no longer required. See
79+
<https://bitcoincore.org/en/lifecycle/#schedule>."
80+
81+
Everyone who commented on the matter in the meeting approved. Luke
82+
Dashjr suggested that if the final commit on each branch wasn't attached
83+
to a release tag (which indicates exactly which code was used for a
84+
particular release) that a tag be added to ensure anyone who needs that
85+
specific code can still obtain it. Other meeting participants agreed.
86+
87+
**Conclusion:** the branches were deleted shortly after the end of the
88+
meeting.
89+
90+
## Moving logging to a separate thread
91+
92+
**Background:** by default, Bitcoin Core writes certain information
93+
about what it's doing to a log file in case something goes wrong and the
94+
user needs to figure out what triggered the problem. Currently, this
95+
logging is done as a sequential step in the program's execution so the
96+
next step after logging isn't taken until the logging completes, this is
97+
described as the logging step "blocking" subsequent steps. Threading is
98+
a way for a program to tell the operating system that multiple steps can
99+
be executed in parallel, which could allow the next step in the program
100+
to start before logging completes (described as "non-blocking").
101+
102+
**Discussion
103+
([log](http://www.erisian.com.au/meetbot/bitcoin-core-dev/2018/bitcoin-core-dev.2018-05-03-19.00.log.html#l-84)):**
104+
James O'Beirne suggested the topic and introduced it by saying, "I think
105+
it may be worthwhile to move logging into a separate thread." He
106+
referenced two recent PRs related to logging, [#13099][] and [#12970][].
107+
108+
Matt Corallo was enthusiastic about the idea, saying
109+
"ACKACKACKACKACKACKACKACKACKACK" and pointing out that, "this is a
110+
surprisingly high lag-creator for miners, at least for those with
111+
spinning-disk-backed-or-cloud-hosted machines." He also linked to a
112+
[commit](https://github.com/bitcoinfibre/bitcoinfibre/commit/6b6a3aef0663775b63bac7d0aa07ec5fc4eb9fc9)
113+
for his project Bitcoin FIBRE (a software fork of Bitcoin Core not used
114+
for consensus enforcement) that implements basic threading for logging.
115+
116+
Several other developers supported the idea and discussion focused
117+
around the best way to implement the behavior and, in particular,
118+
how to ensure that if something does go wrong, as much information as
119+
possible is still written to the log.
120+
121+
Some participants also discussed how much faster Bitcoin Core would be
122+
if logging was moved to a separate thread. General opinion seemed to be
123+
that it could help in some time-critical applications, such as miners
124+
announcing newly-discovered blocks, and would also help when users were
125+
running with optional verbose logging enabled for debugging. The later
126+
is something often done by developers and which is used automatically by
127+
parts of Bitcoin Core's test suite. For other use cases, however, it
128+
was not expected to provide a significant improvement.
129+
130+
**Conclusion:** James O'Beirne finished the discussion by saying,
131+
"unless anyone has any objections, I'll start working on a
132+
thread-consumes-from-ring-buffer implementation in the near future."
133+
134+
## 0.16.1
135+
136+
**Background:** the most recent major version of Bitcoin Core, version
137+
0.16.0, was released about two months ago. Often after a release, bugs
138+
affecting that release are fixed and certain new features are considered
139+
to be important enough to [backport][] to that release, resulting in a
140+
new minor release.
141+
142+
**Discussion
143+
([log](http://www.erisian.com.au/meetbot/bitcoin-core-dev/2018/bitcoin-core-dev.2018-05-03-19.00.log.html#l-188)):**
144+
Matt Corallo request the topic and introduced it, "for those who weren't
145+
paying attention, \[Jesse Cohen] found some particularly novel races in
146+
block handling in [#13092][]. Because they're threading issues, they
147+
almost certainly won't effect anyone except submitblock users, i.e. miners,
148+
and only [in] rare races [cases?] but, still, I think given that and
149+
some of the other various fixes we've had, it may be worth backporting."
150+
151+
The cited issue, #13092, is an analysis by Suhas Daftuar of an issue
152+
uncovered by integration tests Cohen wrote in PR [#13023][]. In
153+
the worst case, a miner could think that they've send a newly-found
154+
block to the network using the `submitblock` RPC only to discover that
155+
Bitcoin Core silently ignored the block because of a [race condition][],
156+
a situation where the program executed steps in a different order than
157+
the programmer expected. Cohen's tests uncover this issue
158+
because they create several test blocks (with no proof of work) in a
159+
very short period of time. There are almost always larger gaps between
160+
blocks on the main network, so hopefully no miners have been affected by
161+
this bug so far.
162+
163+
Although there's a PR to fix the issue, Daftuar believes additional
164+
discussion is needed in order "to settle on the right fix."
165+
Cohen suggested [#12988][] is a "similar type of bug" and should
166+
possibly be fixed in the minor release as well.
167+
168+
Corallo also suggested that 0.16.1 should include PR [#11423][] by
169+
Johnson Lau to make the `CodeSeparator` opcode non-standard in spends of
170+
legacy (non-segwit) inputs. Non-standard means that nodes will not
171+
accept transactions with those inputs into the mempool; they will still
172+
accept them if they occur in a block. This should eliminate the use of
173+
a function called `FindAndDelete()` that has had a problematic history
174+
(see [1][todd-sighash-single], [2][lerner-findandreplace]). Segwit was
175+
implemented [without a need for FindAndDelete](https://github.com/bitcoin/bips/blob/master/bip-0143.mediawiki#No_FindAndDelete)
176+
but still provides the `CodeSeparator` opcode, which NBitcoin developer
177+
Nicholas Dorier has been [using][using codeseparator] as part of a
178+
[Tumblebit][] implementation---so making `CodeSeparator` non-standard in
179+
segwit spends has not been formally proposed.
180+
181+
[using codeseparator]: https://github.com/bitcoin/bitcoin/pull/11423#issuecomment-333439463
182+
183+
There was some discussion about whether Lau's PR was ready for merge.
184+
It was Corallo's belief that Lau "wanted to add one more policy rule [to
185+
Lau's PR]." Corallo said he'll contact Lau about it.
186+
187+
**Conclusion:** all participants seemed in favor of putting together a
188+
0.16.1 release to fix the race condition and include the additional
189+
standardness rule. During and subsequent to the meeting, the various
190+
PRs discussed were added to the project's [0.16.1 milestone][].
191+
192+
## Call ProcessNewBlock asynchronously
193+
194+
**Background:** Bitcoin Core currently processes messages it receives
195+
from its peers on the peer-to-peer network in a single thread. If it
196+
could be rewritten to use multiple threads, it could process some
197+
messages in parallel, which could provide a performance improvement.
198+
However, because it often receives the same basic message from multiple
199+
peers, this presents a challenge of how to avoid doing duplicate work.
200+
201+
**Discussion
202+
([log](http://www.erisian.com.au/meetbot/bitcoin-core-dev/2018/bitcoin-core-dev.2018-05-03-19.00.log.html#l-234)):**
203+
Matt Corallo requested the topic and introduced it, "PR [#12934][] [is]
204+
certainly not ready for review, but we should maybe have a discussion
205+
about what concurrency across peers is gonna look like. There are two
206+
main approaches, but both end up requiring similar refactors for the
207+
majority of their work. In the past I've looked at doing
208+
ProcessMessages() in parallel; in [the previously-linked PR, Jesse Cohen]
209+
moves the validation processing of [transactions and blocks] into a
210+
queue and does that in a separate thread. In both cases, we end up
211+
building logic to 'pause' processing of a peer until whatever message it
212+
just generated has been processed."
213+
214+
Gregory Maxwell and Corallo briefly discussed which parts of the system
215+
would find concurrency especially beneficial. For Maxwell, it was
216+
Initial Block Download (IBD) where download of new blocks is delayed
217+
"when connecting multiple blocks at a time (due to out of order fetching
218+
in IBD)". For Corallo, it would be relaying `gettxn` (get transaction)
219+
requests during connection of a new block received using CompactBlocks
220+
([BIP152][]), "the one big cheapish win left for block-relay-latency
221+
[improvements]."
222+
223+
Corallo then pointed out that concurrency would allow newly-received
224+
data from multiple peers to be simultaneously deserialized into usable
225+
data structures on systems with multiple CPU cores, which Maxwell agreed
226+
could be a nice gain. They also agreed that the many improvements made to
227+
the code in the past year make it simpler and easier to implement this
228+
type of change.
229+
230+
**Conclusion:** Cohen wrote, "Cool---so, if no strong objections
231+
or concerns with the approach, I'll continue this and come back when
232+
it's more ready for review."
233+
234+
## Comic relief
235+
236+
{% highlight text %}
237+
<wumpus> #10740 given me an unicorn though
238+
<jonasschnelli> has also a unicorn, reload solved
239+
<wumpus> yes
240+
<LukeJr> unicorns probably have a high street value
241+
<jnewbery> not any more. The market's been flooded
242+
<LukeJr> shows what I know of unicorn markets
243+
<wumpus> LukeJr: yes, I"m trying to farm them and sell them,
244+
but I have more now than atoms in the knows universe
245+
so you could say the supply is more than the demand...
246+
{% endhighlight %}
247+
248+
## Participants
249+
250+
| IRC nick | Name/Nym |
251+
|-----------------|---------------------------|
252+
| BlueMatt | [Matt Corallo][] |
253+
| wumpus | [Wladimir van der Laan][] |
254+
| gmaxwell | [Gregory Maxwell][] |
255+
| LukeJr | [Luke Dashjr][] |
256+
| skeees | [Jesse Cohen][] |
257+
| jamesob | [James O'Beirne][] |
258+
| jnewbery | [John Newbery][] |
259+
| sipa | [Pieter Wuille][] |
260+
| cfields | [Cory Fields][] |
261+
| MarcoFalke | [Marco Falke][] |
262+
| jonasschnelli | [Jonas Schnelli][] |
263+
| sdaftuar | [Suhas Daftuar][] |
264+
| jimpo | [Jim Posen][] |
265+
| promag | [Joao Barbosa][] |
266+
| jtimon | [Jorge Timón][] |
267+
| achow101 | [Andrew Chow][] |
268+
| jcorgan | [Johnathan Corgan][] |
269+
| kanzure | [Bryan Bishop][] |
270+
271+
## Disclaimer
272+
273+
This summary was compiled without input from any of the participants in
274+
the discussion, so any errors are the fault of the summary author and
275+
not the discussion participants. In particular, quotes taken from the
276+
discussion had their capitalization, punctuation, and spelling modified
277+
to produce consistent sentences. Bracketed words and fragments, as well
278+
as background narratives and exposition, were added by the author of
279+
this summary and may have accidentally changed the meaning of some
280+
sentences. If you believe any quote was taken out of context, please
281+
contact us and we will correct the mistake.
282+
283+
[0.16.1 milestone]: https://github.com/bitcoin/bitcoin/milestone/34
284+
[#10740]: https://github.com/bitcoin/bitcoin/issues/10740
285+
[#11423]: https://github.com/bitcoin/bitcoin/issues/11423
286+
[#12254]: https://github.com/bitcoin/bitcoin/issues/12254
287+
[#12934]: https://github.com/bitcoin/bitcoin/issues/12934
288+
[#12970]: https://github.com/bitcoin/bitcoin/issues/12970
289+
[#12988]: https://github.com/bitcoin/bitcoin/issues/12988
290+
[#13023]: https://github.com/bitcoin/bitcoin/issues/13023
291+
[#13092]: https://github.com/bitcoin/bitcoin/issues/13092
292+
[#13097]: https://github.com/bitcoin/bitcoin/issues/13097
293+
[#13099]: https://github.com/bitcoin/bitcoin/issues/13099
294+
[backport]: https://en.wikipedia.org/wiki/Backporting
295+
[current high-priority PRs]: https://github.com/bitcoin/bitcoin/projects/8
296+
[lerner-findandreplace]: https://bitslog.wordpress.com/2017/01/08/a-bitcoin-transaction-that-takes-5-hours-to-verify/
297+
[race condition]: https://en.wikipedia.org/wiki/Race_condition
298+
[todd-sighash-single]: https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2014-November/006878.html
299+
[tumblebit]: http://cs-people.bu.edu/heilman/tumblebit/

0 commit comments

Comments
 (0)