|
| 1 | +--- |
| 2 | +title: IRC meeting summary for 2018-11-01 |
| 3 | +lang: en |
| 4 | +permalink: /en/meetings/2018/11/01/ |
| 5 | +name: 2018-11-01-meeting |
| 6 | +layout: page |
| 7 | +type: meetings |
| 8 | +version: 1 |
| 9 | +--- |
| 10 | +{% include toc.html %} |
| 11 | +{% include references.md %} |
| 12 | + |
| 13 | +- View this week's log on [MeetBot][mb log] |
| 14 | +- Meeting minutes by [MeetBot][mb minutes] |
| 15 | + |
| 16 | +--- |
| 17 | + |
| 18 | +Topics covered during this weeks contributor meeting included pull requests |
| 19 | +nominated for high priority review, the absence of tests for non-HD wallet code |
| 20 | +paths, a review of the progress made on refactoring the wallet and the spurious |
| 21 | +failures produced by tests run on AppVeyor CI. To complete the meeting, |
| 22 | +contributors were encouraged to share the projects they are actively working |
| 23 | +on. |
| 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][log hipri]):** the following PRs were discussed: |
| 37 | + |
| 38 | +- [#14532][] - Never bind `INADDR_ANY` by default, and warn when doing so |
| 39 | + explicitly by luke-jr |
| 40 | + |
| 41 | +- [#14350][] - Add `WalletLocation` class by promag |
| 42 | + |
| 43 | +- [#14046][] - net: Refactor message parsing (`CNetMessage`) by jonasschnelli |
| 44 | + |
| 45 | +- [#14477][] - Add ability to convert solvability info to descriptor by sipa |
| 46 | + |
| 47 | +- [#13932][] - Additional utility RPCs for PSBT by achow101 |
| 48 | + |
| 49 | +- [#14437][] - Refactor: Start to separate wallet from node by ryanofsky |
| 50 | + |
| 51 | +- [#14336][] - net: implement poll by pstratem |
| 52 | + |
| 53 | +## non-HD Wallet Tests |
| 54 | + |
| 55 | +**Background:** Bitcoin Core [introduced support][bitcoin core bip32] for |
| 56 | +[BIP32][] Hierarchical Deterministic wallets in 0.13.0, greatly simplifying the |
| 57 | +wallet backup and restoration processes. Instead of making regular backups of |
| 58 | +an amorphous keypool, a single backup can be made of the wallet’s extended |
| 59 | +private masterkey (from which all keys in the wallet are deterministically |
| 60 | +generated). Prior to version 0.16.0, users were given the option of disabling |
| 61 | +this feature by specifying `-usehd=0`. When this option was removed, so were the |
| 62 | +tests for non-HD wallet code paths. While non-HD wallets can no longer be |
| 63 | +created by newer versions of Bitcoin Core, they can still be imported. |
| 64 | + |
| 65 | +**Discussion ([log][log nonhdwallets]):** This topic was suggested by luke-jr. |
| 66 | +Because Bitcoin Core no longer creates non-HD wallets, it was mentioned by |
| 67 | +provoostenator, achow101 and jfnewbery that non-HD versions of the wallet would |
| 68 | +have to be packaged into the test framework. Sipa pointed out the importance of |
| 69 | +testing wallet upgrade scenarios (the [failure modes][wallet failure modes] of |
| 70 | +a wallet that is being backed up and restored when Bitcoin Core is being |
| 71 | +upgraded and/or downgraded can lead to irrecoverable loss of funds). |
| 72 | + |
| 73 | +**Conclusion:** Discussion of the various approaches should be continued in |
| 74 | +issue [#14536][]. |
| 75 | + |
| 76 | +## Wallet Refactor |
| 77 | + |
| 78 | +**Background:** The Bitcoin Core wallet is actively being refactored. This work |
| 79 | +has a number of goals, some of which include: improving upon the architectural |
| 80 | +modularity of Bitcoin Core, simplifying the wallet's codebase and introducing |
| 81 | +new features. One project within this broader set of goals is the [output |
| 82 | +descriptors][] language. Introduced in 0.17.0, this language is designed to encode |
| 83 | +the spending requirements of key sets into a single string; this approach to |
| 84 | +classifying unspent outputs improves upon the preexisting [isMine][] logic, the |
| 85 | +complexities and inefficiencies of which impede the development of more |
| 86 | +advanced wallet features (e.g. hardware wallet integration that supports |
| 87 | +[PSBT][]-compatible key signing protocols). |
| 88 | + |
| 89 | +**Discussion ([log][log walletrefactor]):** In [#14565][], Sipa continues his work |
| 90 | +overhauling the logic of the importmulti RPC to restrict superfluous key or |
| 91 | +script data from being imported; this is necessary to support "old style" |
| 92 | +descriptor imports that implicitly convert the descriptor into existing wallet |
| 93 | +structures ([#14491][]). Sipa will also tackle some pubkey-caching prep work |
| 94 | +required to use descriptors instead of keypools. To support native descriptor |
| 95 | +imports, the existing keypool/isMine logic must be refactored to sit behind an |
| 96 | +abstraction that can be instantiated by old wallet logic or descriptors. |
| 97 | + |
| 98 | +**Conclusion:** Meshcollider will rebase #14491 when #14565 is complete. |
| 99 | +Achow101 will rebase [#14075][] on top of #14491 thereafter. #14705 enables a |
| 100 | +wallet with --disable-private-keys set to import and fetch pubkeys to/from the |
| 101 | +keypool. This aids in the "[disentanglement][wallet stuff]" of watch-only wallets from wallets |
| 102 | +that store private keys. Without this PR, a wallet with disabled private keys |
| 103 | +will not fetch pubkeys from the keypool. This is useful for hardware wallets |
| 104 | +that interact with Bitcoin Core as an external signer; to support this use |
| 105 | +case, the keypool must accept the importation and retrieval of watch-only |
| 106 | +pubkeys generated by the hardware wallet. Discussion was concluded with the |
| 107 | +suggestion that a status update from ryanofsky on his [code][#10973] |
| 108 | +[separation][#14437] PRs be saved for the second Bitcoin Core Wallet |
| 109 | +Contributor meeting. |
| 110 | + |
| 111 | +**Note:** During the “[wallet stuff][wallet stuff]” session at the |
| 112 | +[CoreDev][coredev] meeting in Tokyo, it was proposed that a separate |
| 113 | +wallet-focused meeting be organized. On Friday [October 19th][walletmeeting1], |
| 114 | +the first Bitcoin Core Wallet Meeting was held. It is scheduled for every other |
| 115 | +Friday at 19:00:00 UTC. The second meeting was held on [November |
| 116 | +2nd][walletmeeting2]. |
| 117 | + |
| 118 | +## AppVeyor Failures |
| 119 | + |
| 120 | +**Background:** AppVeyor is a hosted continuous integration service that |
| 121 | +provides Microsoft Visual C++ (MSVC) build environments to Bitcoin Core. |
| 122 | +Binaries produced in these environments are for cross-platform testing purposes |
| 123 | +only. |
| 124 | + |
| 125 | +**Discussion ([log][log appveyor]):** The functional tests run on AppVeyor fail |
| 126 | +spuriously. This frustrates some of the contributors. Sipa requests that this |
| 127 | +issue be addressed. |
| 128 | + |
| 129 | +**Conclusion:** There is an open issue [#14446][] which details the failure |
| 130 | +types. PR [#13501][] may mitigate one class of these failures. |
| 131 | + |
| 132 | +## What are people working on? |
| 133 | + |
| 134 | +**[log][log workingon]** |
| 135 | + |
| 136 | +- jarthur: UNIX domain sockets for the RPC API. [Previous |
| 137 | + discussion][unixsocketsdiscussion]. [Open issue][unixsocketissue]. |
| 138 | +- luke-jr: rebasing his PRs |
| 139 | +- achow101: PSBT and hardware wallet integration |
| 140 | +- jnewbery: spend more time reviewing wallet PRs by the end of the year |
| 141 | +- instagibbs: reviewing wallet PRs and hardware wallet integration |
| 142 | +- sipa: private authentication protocol for P2P network. [Previous |
| 143 | + discussion][countersigndiscussion]. [More reading][countersigngist] (note: |
| 144 | + there are known vulnerabilities with the proposed protocol). |
| 145 | +- meshcollider: wallet refactor and review |
| 146 | + |
| 147 | +## Participants |
| 148 | + |
| 149 | +| IRC nick | Name/Nym | |
| 150 | +|-----------------|---------------------------| |
| 151 | +| sipa | [Pieter Wuille][] | |
| 152 | +| provoostenator | [Sjors Provoost][] | |
| 153 | +| luke-jr | [Luke Dashjr][] | |
| 154 | +| achow101 | [Andrew Chow][] | |
| 155 | +| MarcoFalke | [Marco Falke][] | |
| 156 | +| meshcollider | [Samuel Dobson][] | |
| 157 | +| jnewbery | [John Newbery][] | |
| 158 | +| instagibbs | [Gregory Sanders][] | |
| 159 | +| phantomcircuit | [Patrick Strateman][] | |
| 160 | +| jarthur | [Justin Arthur][] | |
| 161 | +| kanzure | [Bryan Bishop][] | |
| 162 | +| gwillen | [Glenn Willen][] | |
| 163 | +| ryanofsky | [Russell Yanofsky][] | |
| 164 | + |
| 165 | +## Disclaimer |
| 166 | + |
| 167 | +This summary was compiled without input from any of the participants in |
| 168 | +the discussion, so any errors are the fault of the summary author and |
| 169 | +not the discussion participants. In particular, quotes taken from the |
| 170 | +discussion had their capitalization, punctuation, and spelling modified |
| 171 | +to produce consistent sentences. Bracketed words and fragments, as well |
| 172 | +as background narratives and exposition, were added by the author of |
| 173 | +this summary and may have accidentally changed the meaning of some |
| 174 | +sentences. If you believe any quote was taken out of context, please |
| 175 | +[open an issue][] and we will correct the mistake. |
| 176 | + |
| 177 | +[current high-priority PRs]: https://github.com/bitcoin/bitcoin/projects/8 |
| 178 | +[open an issue]: https://github.com/bitcoin-core/bitcoincore.org/issues/new |
| 179 | + |
| 180 | +[mb minutes]: http://www.erisian.com.au/meetbot/bitcoin-core-dev/2018/bitcoin-core-dev.2018-11-01-19.04.html |
| 181 | +{% assign log = "http://www.erisian.com.au/meetbot/bitcoin-core-dev/2018/bitcoin-core-dev.2018-11-01-19.04.log.html" %} |
| 182 | +[mb log]: {{log}} |
| 183 | +[log hipri]: {{log}}#l-16 |
| 184 | +[log nonhdwallets]: {{log}}#l-56 |
| 185 | +[log walletrefactor]: {{log}}#l-86 |
| 186 | +[log appveyor]: {{log}}#l-131 |
| 187 | +[log workingon]: {{log}}#l-151 |
| 188 | + |
| 189 | +[bitcoin core bip32]: https://bitcoincore.org/en/2016/08/23/release-0.13.0/#bip32-hd-wallet-support |
| 190 | +[wallet failure modes]: https://gist.github.com/sipa/125cfa1615946d0c3f3eec2ad7f250a2#segwit-wallet-support |
| 191 | +[output descriptors]: https://github.com/bitcoin/bitcoin/blob/master/doc/descriptors.md |
| 192 | +[isMine]: https://gist.github.com/sipa/125cfa1615946d0c3f3eec2ad7f250a2#current-design |
| 193 | +[PSBT]: https://github.com/bitcoin/bitcoin/blob/master/doc/psbt.md |
| 194 | +[wallet stuff]: http://diyhpl.us/wiki/transcripts/bitcoin-core-dev-tech/2018-10-09-wallet-stuff/ |
| 195 | +[coredev]: https://coredev.tech/ |
| 196 | +[walletmeeting1]: http://www.erisian.com.au/meetbot/bitcoin-core-dev/2018/bitcoin-core-dev.2018-10-19-19.04.log.html |
| 197 | +[walletmeeting2]: http://www.erisian.com.au/meetbot/bitcoin-core-dev/2018/bitcoin-core-dev.2018-11-02-19.01.log.html |
| 198 | +[unixsocketsdiscussion]: http://www.erisian.com.au/bitcoin-core-dev/log-2018-10-25.html#l-939 |
| 199 | +[unixsocketissue]: https://github.com/bitcoin/bitcoin/issues/9919 |
| 200 | +[countersigndiscussion]: http://www.erisian.com.au/meetbot/bitcoin-core-dev/2018/bitcoin-core-dev.2018-04-19-19.00.log.html#l-200 |
| 201 | +[countersigngist]: https://gist.github.com/sipa/d7dcaae0419f10e5be0270fada84c20b |
| 202 | + |
| 203 | +{% include link-to-issues.md issues="10973,13501,13932,14046,14075,14336,14350,14437,14446,14477,14491,14532,14536,14565" %} |
0 commit comments