-
Notifications
You must be signed in to change notification settings - Fork 458
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
- Loading branch information
Showing
2 changed files
with
278 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,274 @@ | ||
| --- | ||
| title: IRC meeting summary for 2018-04-12 | ||
| permalink: /en/meetings/2018/04/12/ | ||
| name: 2018-04-12-meeting | ||
| type: meetings | ||
| layout: page | ||
| lang: en | ||
| version: 1 | ||
| --- | ||
| {% include _toc.html %} | ||
| {% include _references.md %} | ||
|
|
||
| - [Link to this week logs](https://botbot.me/freenode/bitcoin-core-dev/2018-04-12/?msg=98918663&page=4) | ||
| - [Meeting minutes by meetbot](http://www.erisian.com.au/meetbot/bitcoin-core-dev/2018/bitcoin-core-dev.2018-04-12-19.01.html) | ||
|
|
||
| --- | ||
|
|
||
| Topics discussed during this weekly meeting included what pull requests | ||
| members of the project would like reviewers to focus on during the | ||
| upcoming week, whether or not to break compatibility with an odd and | ||
| rarely used feature in the wallet (IsMine bare multisig), how to safely | ||
| improve multiwallet support, and how to deal with some unsupported | ||
| software when upgrading the build environment for Bitcoin Core's | ||
| reproducible binary releases. | ||
|
|
||
| ## 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](https://github.com/bitcoin/bitcoin/projects/8). | ||
|
|
||
| *Discussion:* The following PRs were suggested for attention this week: | ||
|
|
||
| 1. **Build tx index in parallel with validation ([#11857][])** nominated | ||
| by Jim Posen and Wladimir van der Laan. This PR moves the optional | ||
| transaction index that allows users to look up historical | ||
| transactions by their txid into a separate database. This will allow | ||
| users to enable or disable the transaction index while a node is | ||
| running instead of having to wait for up to several hours even on | ||
| fast hardware. The PR also lays the groundwork for adding additional | ||
| separate indices to Bitcoin Core that can enable features such as | ||
| improved-privacy block filtering for lightweight clients as described | ||
| in BIPs [157][BIP157] and [158][BIP158]. | ||
|
|
||
| Prior to the meeting, the PR has already received some positive | ||
| review but a few changes were requested and have now been made, so | ||
| discussion in the meeting focused on getting those changes | ||
| themselves reviewed. | ||
|
|
||
| 2. **Upgrade path for non-HD wallets to HD ([#12560][])** nominated by | ||
| Van der Laan. This PR adds a new `sethdseed` (set HD seed) RPC that | ||
| allows users to set the seed value for a [BIP32][] Hierarchical | ||
| Deterministic (HD) wallet. This can be used to request Bitcoin Core | ||
| generate a new seed on its own or to change the seed to a value | ||
| obtained from outside the currently-running Bitcoin Core (such as | ||
| from a backup). In addition, the PR allows users of older non-HD | ||
| Bitcoin Core wallets to upgrade to HD wallets using the | ||
| `-upgradewallet` command line parameter. | ||
|
|
||
| Discussion in the meeting indicated that the PR may have one | ||
| unaddressed issue but that the PR primarily needs review from | ||
| additional contributors. | ||
|
|
||
| 3. **Move fee estimator into validationinterface/cscheduler thread | ||
| ([#11775][])** nominated by Van der Laan. This PR makes a backend | ||
| change to some internal interfaces in Bitcoin Core's code and then | ||
| changes Bitcoin Core's fee estimator to use one of those interfaces, | ||
| giving it access to additional information useful for fee estimation. | ||
| The PR also makes some minor improvements to the fee estimator in the | ||
| way that it handles edge cases. | ||
|
|
||
| Prior to the meeting, the author of the PR (Matt Corallo) had | ||
| offered to split the PR into separate smaller PRs for the different | ||
| parts of the change. Discussion during the meeting indicated that | ||
| reviewers are waiting for this to happen before reviewing further. | ||
|
|
||
| 4. **Introduce `getblockstats` RPC to [provide data that can be used to] | ||
| plot things ([#10757][])** nominated by Jorge Timón. This PR adds a | ||
| new RPC that returns various details and statistics about a specified | ||
| block. | ||
|
|
||
| Discussion in the meeting saw one contributor agreeing to review the | ||
| PR. | ||
|
|
||
| A concern mentioned during the discussion was GitHub's recent problem of | ||
| pages failing to load for significant periods of time. The problem has | ||
| become frequent enough that meeting participants mention the problem | ||
| by reference to the drawing GitHub displays on the error page, a | ||
| unicorn. | ||
|
|
||
| ## What to do with IsMine on bare multisig | ||
|
|
||
| *Background:* Bare multsig refers to a payment to multiple public keys | ||
| without using a much more commonly used P2SH or segwit address. Bitcoin | ||
| Core's wallet currently scans any bare multisig payments to see if every | ||
| public key used is part of the user's wallet and, if so, categorizes the | ||
| payment as *InMine* to indicate that it's spendable by the user. | ||
|
|
||
| *Discussion:* Pieter Wuille requested the topic. He described the | ||
| current behavior as, "stupid, annoying, pointless, and hard to | ||
| maintain." For *stupid* and *pointless,* the issue is probably that | ||
| multisig doesn't provide any additional security over single-sig when | ||
| all of the public keys belong to the same wallet, but using multisig | ||
| does cost more than single-sig. For *annoying* and *hard to maintain,* | ||
| the issue is that this behavior requires extra code and is making it more | ||
| difficult for developers to work towards an [improved wallet | ||
| design](https://gist.github.com/sipa/125cfa1615946d0c3f3eec2ad7f250a2) | ||
| previously described by Wuille. | ||
|
|
||
| Wuille then described his concern: "there may be existing outputs to it. | ||
| I don't know if this is the case or not, but it sounds scary to just | ||
| stop supporting it." After some discussion, Wuille clarified that he | ||
| isn't proposing to remove the code that allows owners of those payments | ||
| to spend them; rather, it would be the case that Bitcoin Core would no | ||
| longer automatically see those payments. | ||
|
|
||
| Additional discussion between Matt Corallo and Wuille provided a vivid | ||
| description of the problem: to support this old behavior in the desired | ||
| new wallet model would require generating N<sup>3</sup> combinations, | ||
| where *N* is the number of private keys the user owns. By default, | ||
| Bitcoin Core generates 2,000 private keys, so the wallet would need to | ||
| generate an unmanageable eight billion combinations. | ||
|
|
||
| Continued discussion surrounded Wuille's PR [#12874][] which disables | ||
| the current behavior and provides a workaround for users who need it. | ||
| Corallo mentioned that the existing RPC `importaddress` already provides | ||
| the important features necessary for the workaround. | ||
|
|
||
| *Conclusion:* although meeting participants were in favor of preserving | ||
| existing functionality if there was a reasonable way to do so, | ||
| discussion seemed to favor removing IsMine on bare multisig and | ||
| mentioning the workaround in the release notes. | ||
|
|
||
| ## Dynamic wallet load/unload | ||
|
|
||
| *Background:* Old versions of Bitcoin Core could only use a single | ||
| instance of the built in wallet with a particular node. This was | ||
| extended to allow multiple wallet instances in [version | ||
| 0.15.0](/en/releases/0.15.0/#multi-wallet-support), but older parts of | ||
| Bitcoin Core (such as command-line options read on startup) assume | ||
| the user only has one wallet instance and so they act on all loaded | ||
| wallets at once. John Newbery's pull request [#10740][] provides a | ||
| workaround for this by allowing users to load, unload, and maybe even | ||
| create wallets at run time using new proposed RPCs. | ||
|
|
||
| *Discussion:* Joao Barbosa requested the topic, suggesting that | ||
| "...wallet management should be with shared pointers". This would help | ||
| ensure that if a user requests to unload a wallet at run time, pending | ||
| requests involving that wallet can be handled before the wallet is | ||
| unloaded. The alternative could be a wallet disappears unexpectedly in | ||
| the middle of an operation, which could cause unexpected and harmful | ||
| effects. Barbosa has opened PR [#11402][] to make this proposed | ||
| change. | ||
|
|
||
| This proposal seemed uncontroversial and discussion moved to other | ||
| improvements and the sequence in which those improvements should be | ||
| implemented. Jonas Schnelli suggested that Bitcoin Core first needed a | ||
| `createwallet` RPC so that wallets could be created at run time and the | ||
| command-line wallet options could be retired (or possibly restricted to | ||
| just single-wallet use). Pieter Wuille noted that a runtime `createwallet` | ||
| would be strange without a runtime way to load a wallet, as you'd have to | ||
| restart Bitcoin Core in order to use the wallet you just created. | ||
|
|
||
| Luke Dashjr suggested the sequence "load -> create -> unload" as "unload | ||
| is the complex part," likely because of the potential problems dealing | ||
| with multiple processes working on a loaded wallet simultaneously. | ||
| Newbery concurred and responded favorably to Schnelli's suggestion | ||
| to "split unload away from the existing PR." | ||
|
|
||
| Wladimir van der Laan suggested that "unload should probably be in two | ||
| stages: after requesting it, RPC and the GUI lose access to it. Then it | ||
| waits for current operations to finish. Then the thing really gets | ||
| [unloaded]." | ||
|
|
||
| *Conclusion:* Newbery agreed that he'll modify his PR, saying on the PR | ||
| itself that he'll "reduce the scope of this PR to just a loadwallet RPC. | ||
| A createwallet RPC should come next, followed by unloadwallet. | ||
| (unloadwallet is where most of the difficulties are)." | ||
|
|
||
| ## Gitian update | ||
|
|
||
| *Background:* Bitcoin Core uses a system called [Gitian][] to build its | ||
| release binaries in a way that is fully reproducible by anyone with a | ||
| computer and Internet connection, allowing anyone to verify that the | ||
| release binaries are the product of the peer-reviewed source code. As | ||
| Bitcoin Core changes, the operating system targeted for building changes, | ||
| and the availability of other software changes, the Gitian build | ||
| environment needs to be updated to handle the changes. | ||
|
|
||
| *Discussion:* Luke Dashjr requested the topic, which seemed to be | ||
| related to the project's desire to switch the operating system used by | ||
| Gitian to the upcoming Ubuntu 18.04 LTS. Dashjr's concern is that "we | ||
| need a replacement for vmbuilder or something, since Canonical hasn't | ||
| updated it to support anything recent." Vmbuilder is a tool that allows | ||
| one to create and run a subsidiary operating system under your regular | ||
| operating system in order to build software in it. A desirable feature | ||
| of vmbuilder is that it can create the sub-operating system in a virtual | ||
| machine to fully isolate it from your main operating system, helping to | ||
| prevent any problems in the code you're building from affecting your | ||
| actual operating system. | ||
|
|
||
| Wladimir van der Laan suggested using debbootstrap (Debian bootstrap), a | ||
| tool that predates vmbuilder and which uses a chroot instead of a | ||
| virtual machine, allowing it to trick normal software into thinking it's | ||
| being built in a separate operating system but which does nothing | ||
| significant to prevent malicious software from attacking the primary | ||
| operating system. Although other developers such as Cory Fields were in | ||
| favor of moving to debootstrap, Dashjr remained concerned and said, "I | ||
| suppose fixing vmbuilder might be not too unreasonable [an] effort, maybe I | ||
| will try that." | ||
|
|
||
| Andrew Chow added that he was "considering adding Docker support to Gitian | ||
| so we would use a default Ubuntu docker image and then build from | ||
| there." Docker can be more secure than a chroot but is usually less | ||
| secure than a virtual machine. Dashjr noted that Docker also restricted to | ||
| the x86_64 platform used by most desktops and servers, whereas some | ||
| project contributor believe it would be advantageous for some of the | ||
| reproducible builds to be created on other platforms that may not have | ||
| the problems found on x86_64 such as those related to the [Intel | ||
| Management Engine](https://en.wikipedia.org/wiki/Intel_Management_Engine). | ||
|
|
||
| *Conclusion:* a switch to debootstrap in parallel with Dashjr | ||
| potentially working on getting vmbuilder working again. Long term, | ||
| Fields is working to overhaul the system. | ||
|
|
||
| ## Comic relief | ||
|
|
||
| {% highlight text %} | ||
| <wumpus> #topic dynamic wallet load/unload (promag) | ||
| <instagibbs_> what's the controversy in this topic :) | ||
| <sipa> it should happen, duh | ||
| <sipa> how and when is another :) | ||
| {% endhighlight %} | ||
|
|
||
| ## Participants | ||
|
|
||
|
|
||
| | IRC nick | Name/Nym | | ||
| |-----------------|---------------------------| | ||
| | sipa | [Pieter Wuille][] | | ||
| | wumpus | [Wladimir van der Laan][] | | ||
| | jonasschenlli | [Jonas Schnelli][] | | ||
| | luke-jr | [Luke Dashjr][] | | ||
| | BlueMatt | [Matt Corallo][] | | ||
| | promag | [Joao Barbosa][] | | ||
| | jnewbery | [John Newbery][] | | ||
| | jtimon | [Jorge Timón][] | | ||
| | jimpo | [Jim Posen][] | | ||
| | achow101 | [Andrew Chow][] | | ||
| | randolf | [Randolf Richardson][] | | ||
| | instagibbs | [Gregory Sanders][] | | ||
| | cfields | [Cory Fields][] | | ||
| | sdaftuar | [Suhas Daftuar][] | | ||
| | meshcollider | [Samuel Dobson][] | | ||
| | jcorgan | [Johnathan Corgan][] | | ||
| | kanzure | [Bryan Bishop][] | | ||
|
|
||
| ## 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. | ||
|
|
||
| [#11857]: https://github.com/bitcoin/bitcoin/issues/11857 | ||
| [#12560]: https://github.com/bitcoin/bitcoin/issues/12560 | ||
| [#11775]: https://github.com/bitcoin/bitcoin/issues/11775 | ||
| [#10757]: https://github.com/bitcoin/bitcoin/issues/10757 | ||
| [#12874]: https://github.com/bitcoin/bitcoin/issues/12874 | ||
| [#10740]: https://github.com/bitcoin/bitcoin/issues/10740 | ||
| [#11402]: https://github.com/bitcoin/bitcoin/issues/11402 | ||
| [Gitian]: https://github.com/bitcoin-core/docs/blob/master/gitian-building.md |