|
| 1 | +--- |
| 2 | +title: IRC meeting summary for 2018-04-26 |
| 3 | +permalink: /en/meetings/2018/04/26/ |
| 4 | +name: 2018-04-26-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/99439716/) or [MeetBot](http://www.erisian.com.au/meetbot/bitcoin-core-dev/2018/bitcoin-core-dev.2018-04-26-19.00.log.html) |
| 14 | +- [Meeting minutes by MeetBot](http://www.erisian.com.au/meetbot/bitcoin-core-dev/2018/bitcoin-core-dev.2018-04-19-19.00.html) |
| 15 | + |
| 16 | +--- |
| 17 | + |
| 18 | +Topics discussed during this weekly meeting included whether or not |
| 19 | +"high priority for review" pull requests are getting enough review, what |
| 20 | +pull requests members of the project would like reviewers to focus on |
| 21 | +during the upcoming week, what to do with a particular parameter to the |
| 22 | +`bumpfee` RPC in an upgrade of that RPC, and whether or not to remove |
| 23 | +Bitcoin Core's disabled-by-default "safe mode". |
| 24 | + |
| 25 | +## High priority for review |
| 26 | + |
| 27 | +**Background:** each meeting, Bitcoin Core developers discuss which Pull |
| 28 | +Requests (PRs) the meeting participants think most need review in the |
| 29 | +upcoming week. Some of these PRs are related to code that contributors |
| 30 | +especially want to see in the next release; others are PRs that are |
| 31 | +blocking further work or which require significant maintenance (rebasing) |
| 32 | +to keep in a pending state. Any capable reviewers are encouraged to |
| 33 | +visit the project's list of [current high-priority |
| 34 | +PRs][]. |
| 35 | + |
| 36 | +**Discussion ([log](http://www.erisian.com.au/meetbot/bitcoin-core-dev/2018/bitcoin-core-dev.2018-04-26-19.00.log.html#l-16)):** |
| 37 | +prior to discussing particular issues, Matt Corallo raised a concern, |
| 38 | +"we haven't really been getting any review on the 'high priority' list, so |
| 39 | +[I'm] not sure the use of bringing it up every week." Other contributors |
| 40 | +agreed that items on the list haven't received much review in the past |
| 41 | +several weeks, but many comments seemed to be in favor of keeping the |
| 42 | +list. The topic was not explicitly resolved, but it seems that the list |
| 43 | +will continue to be used for at least another week. |
| 44 | + |
| 45 | +The issue of some GitHub pages not loading ("unicorns") as mentioned in |
| 46 | +the previous two weekly meeting summaries was briefly mentioned in this |
| 47 | +meeting again. |
| 48 | + |
| 49 | +Specific PRs discussed during the meeting include, |
| 50 | + |
| 51 | +1. **Split validationinterface into parallel validation/mempool |
| 52 | + interfaces ([#12979][]),** which was previously added to the list. |
| 53 | + This PR splits up logic related to validating transactions coming |
| 54 | + into the mempool into separate validation and mempool interfaces, |
| 55 | + making it easier to get certain information about the mempool and |
| 56 | + laying the groundwork for potential improvements to Bitcoin Core's |
| 57 | + fee estimator. |
| 58 | + |
| 59 | + Pieter Wuille noted that he "started reviewing #12979 but had |
| 60 | + difficulty following [it]." The PR author, Corallo, said that "it's |
| 61 | + a pure refactor. All it is [doing] is moving things around." |
| 62 | + Corallo also mentioned that it's on the high-priority list because |
| 63 | + it's "blocking about 10 other things." |
| 64 | + |
| 65 | +1. **Introduce `getblockstats` RPC to [provide data that can be used to] |
| 66 | + plot things ([#10757][]),** previously nominated by Jorge Timón. |
| 67 | + This PR adds a new RPC that returns various details and statistics |
| 68 | + about a specified block. |
| 69 | + |
| 70 | + Anthony Towns noted that this PR received some review during the |
| 71 | + previous week but that it still had some minor outstanding issues. |
| 72 | + |
| 73 | +## The necessity of "totalFee" as an argument for bumpfee |
| 74 | + |
| 75 | +**Background:** Bitcoin Core provides a `bumpfee` RPC to allow |
| 76 | +increasing ("bumping") the transaction fee paid on any of the user's |
| 77 | +unconfirmed transactions that signal [BIP125][] opt-in Replace-By-Fee |
| 78 | +(RBF) transaction replacability. By default, this RPC calculates the |
| 79 | +amount of the increase on its own, but it also allows the user to |
| 80 | +optionally specify the new fee to pay using the `totalFee` parameter. |
| 81 | + |
| 82 | +**Discussion ([log](http://www.erisian.com.au/meetbot/bitcoin-core-dev/2018/bitcoin-core-dev.2018-04-26-19.00.log.html#l-87)):** Gregory Sanders requested the topic and |
| 83 | +asked, "is [the `totalFee` parameter] needed? [...] I was hoping to |
| 84 | +upgrade RBF/CPFP [Child Pays For Parent] in the not too distant future, but |
| 85 | +it complicates [the] logic to support [this parameter]." |
| 86 | + |
| 87 | +Anthony Towns suggested the parameter (or a replacement) could be used |
| 88 | +instead as an upper bound on how much fee the user was willing to pay |
| 89 | +and the RPC could error out if the automatically-determined increase |
| 90 | +value exceeded that ceiling. |
| 91 | + |
| 92 | +Sanders further described his motivation for wanting to drop the |
| 93 | +parameter: "I redid [`bumpfee`] to just use `CreateTransaction` so it |
| 94 | +will select more coins, which changes size." In other words, adding |
| 95 | +more inputs (coins) to the replacement transaction makes the replacement |
| 96 | +larger than the original transaction and requires paying additional fees |
| 97 | +to cover the increased size---but the user won't know that at the time |
| 98 | +they call `bumpfee` with a specific `totalFee` parameter. |
| 99 | + |
| 100 | +Luke Dashjr, Suhas Daftuar, and Pieter Wuille all seemed to agree that |
| 101 | +`totalFee` doesn't seem to make sense when changing the size of the |
| 102 | +transaction. |
| 103 | + |
| 104 | +**Conclusion:** no explicit conclusion. Sanders closed the topic by |
| 105 | +suggesting he could design the upgraded `bumpfee` to use the new |
| 106 | +behavior of allowing additional inputs by default but optionally |
| 107 | +supporting the old behavior when `totalFee` is used where additional |
| 108 | +inputs will not be added and the RPC will fail when the current inputs |
| 109 | +aren't sufficient to pay for the desired fee increase. That would be |
| 110 | +"backwards compatible without additional cruft." |
| 111 | + |
| 112 | +## Remove safemode |
| 113 | + |
| 114 | +**Background:** an early version of the Bitcoin Core software (when it |
| 115 | +was called "Bitcoin") introduced a "safe mode" that disabled certain |
| 116 | +RPCs during network disruptions in order to try to prevent users from |
| 117 | +losing money. The criteria that triggered safe mode have changed over |
| 118 | +the years, new RPCs were rarely added to the list that were disabled in |
| 119 | +safe mode, and eventually Bitcoin Core 0.16 disabled safe mode by |
| 120 | +default. |
| 121 | + |
| 122 | +**Discussion ([log](http://www.erisian.com.au/meetbot/bitcoin-core-dev/2018/bitcoin-core-dev.2018-04-26-19.00.log.html#l-129)):** Wladimir van der Laan requested the topic, |
| 123 | +referenced PR [#10563][] by Andrew Chow which Van der Laan rebased |
| 124 | +as PR [#13090][], and asked, "safemode was disabled since 0.16; should |
| 125 | +we completely remove it for 0.17?" |
| 126 | + |
| 127 | +Several contributors said they weren't aware of anyone using it and |
| 128 | +didn't think it's currently useful. Pieter Wuille said, "disabling RPCs |
| 129 | +is not how the Bitcoin ecosystem will deal with an emergency anyway---a |
| 130 | +lot of infrastructure wouldn't even notice." |
| 131 | + |
| 132 | +Van der Laan noted that Bitcoin Core will continue to provide an |
| 133 | +`-alertnotify` startup parameter which can be used to run an arbitrary |
| 134 | +script when safemode would have activated. Luke Dashjr suggested that |
| 135 | +`-alertnotify` could be used with a proposed upcoming RPC, |
| 136 | +`walletunload`, to disable the wallet during an emergency; that would be |
| 137 | +similar (and possibly superior) to the current safemode design. |
| 138 | + |
| 139 | +**Conclusion:** although a few participants wished Bitcoin Core could be |
| 140 | +better at detecting disruptive network conditions and could |
| 141 | +automatically do something to help users avoid losing money, all |
| 142 | +participants seemed to be in favor of removing the current unsupported |
| 143 | +safe mode system. Subsequent to the meeting, #13090 was merged to remove |
| 144 | +safe mode from the development branch. |
| 145 | + |
| 146 | +Note, during the discussion for removing safemode, the suggestion to |
| 147 | +combine `-alertnotify` with the proposed new RPC `walletunload` was |
| 148 | +misinterpreted as a request to discuss the work currently being done on |
| 149 | +`walletunload`. This is why "walletunload" shows up as a topic in the |
| 150 | +MeetBot meeting summary even though it was not directly discussed. |
| 151 | + |
| 152 | +## Mini-topics |
| 153 | + |
| 154 | +One briefly discussed topic was an update on signing certificates for |
| 155 | +Bitcoin Core binaries since Cory Fields's |
| 156 | +[email](https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2018-January/015542.html) |
| 157 | +about the subject to the mailing list in January. In that email, Fields |
| 158 | +said Gregory Maxwell was working on "establishing a new threshold |
| 159 | +signing scheme that will allow us to handle code signing without any |
| 160 | +single point of failure." No update was available as Maxwell was not |
| 161 | +present for the meeting. |
| 162 | + |
| 163 | +## Comic relief |
| 164 | + |
| 165 | +{% highlight text %} |
| 166 | +<wumpus> #topic walletunload (Lukejr) |
| 167 | +<LukeJr> wumpus: I wasn't suggesting it as a topic |
| 168 | +<wumpus> LukeJr: oh... |
| 169 | + <sipa> #unload walletunload |
| 170 | +<wumpus> #untopic |
| 171 | +{% endhighlight %} |
| 172 | + |
| 173 | +## Participants |
| 174 | + |
| 175 | +| IRC nick | Name/Nym | |
| 176 | +|-----------------|---------------------------| |
| 177 | +| wumpus | [Wladimir van der Laan][] | |
| 178 | +| instagibbs | [Gregory Sanders][] | |
| 179 | +| BlueMatt | [Matt Corallo][] | |
| 180 | +| luke-jr | [Luke Dashjr][] | |
| 181 | +| sipa | [Pieter Wuille][] | |
| 182 | +| sdaftuar | [Suhas Daftuar][] | |
| 183 | +| jonasschnelli | [Jonas Schnelli][] | |
| 184 | +| achow101 | [Andrew Chow][] | |
| 185 | +| aj | [Anthony Towns][] | |
| 186 | +| promag | [Joao Barbosa][] | |
| 187 | +| fanquake | [Michael Ford][] | |
| 188 | +| jamesob | [James O'Beirne][] | |
| 189 | +| jtimon | [Jorge Timón][] | |
| 190 | +| cfields | [Cory Fields][] | |
| 191 | +| kanzure | [Bryan Bishop][] | |
| 192 | + |
| 193 | +## Disclaimer |
| 194 | + |
| 195 | +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. |
| 196 | + |
| 197 | +[#12979]: https://github.com/bitcoin/bitcoin/issues/12979 |
| 198 | +[#10757]: https://github.com/bitcoin/bitcoin/issues/10757 |
| 199 | +[#10563]: https://github.com/bitcoin/bitcoin/issues/10563 |
| 200 | +[#13090]: https://github.com/bitcoin/bitcoin/issues/13090 |
| 201 | +[current high-priority PRs]: https://github.com/bitcoin/bitcoin/projects/8 |
| 202 | + |
0 commit comments