IRC meeting summary for 2018-04-12

• Link to this week logs
• Meeting minutes by meetbot

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.

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 and 158.

   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 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³ 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, 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.

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

<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 :)

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.