156 Commits

Author SHA1 Message Date
Antoine Poinsot
8b129fe3e5
Merge #74: Decent change management (using multipath descriptors)
117171f24ff5bd6731d9e5e50e4515a03013a9eb commands: use a separate key chain for change addresses (Antoine Poinsot)
d9f905a19a5c6076683bbe7714ecb86fbafe0555 db: track the next unused derivation index for change, too (Antoine Poinsot)
58a0e57c59bf7b4f451580917f701e95e45d1af6 db: record whether a coin was received on a change address (Antoine Poinsot)
9b04a551474b2cd5ed793e42832454635d964495 db: store derivation index also for addresses from the change desc (Antoine Poinsot)
4f3daa7741b6996c17133dacedb4e70c66f5bac8 descriptors: cache the receive and change descriptors (Antoine Poinsot)
ca3d7c1f3360daa53fbec723fb960051a926d687 descriptors: introduce a newtype for the multipath descriptor (Antoine Poinsot)
1320ee30bacdedd449dfa89a3008e01995917c9f daemon: use multipath descriptors (Antoine Poinsot)
d4db804e4bad928466fa1db4a84a474ff4c33d7b qa: add a missing 'wait_for' in spend creation test (Antoine Poinsot)
7a18c583cbfcd958db9c79e8b13a0a68c1d20d41 bitcoind: filter received coins based on parent descriptors (Antoine Poinsot)
ba4c1e0383e302d2a822176d93b3b9fadc174b4b bitcoind: include change outputs in listsinceblock (Antoine Poinsot)
caaca1fd1a721acae150b8bd9212e51b8e378c99 descriptors: rename derive into derive_received (Antoine Poinsot)
f985fd787917e344b17ae90edd7b99bc1c9f3a7c descriptors: remove as_inner method (Antoine Poinsot)
846d924792089e41e530e414b26f0823afc151eb qa: upgrade python-bip380 to latest master (Antoine Poinsot)
3105b86a28444097e20e14261ffbfd5448b2854b Use my own fork of rust-miniscript (Antoine Poinsot)

Pull request description:

  This fixes #18 by implementing the de-facto standard of using a `/0/*` keychain for receiving addresses and a `/1/*` keychain for change addresses. Note that once we'll have multisig, reusing addresses will still be possible since wallet don't share the same "next derivation index".

  In order to avoid forcing the user to configure and backup two almost identical descriptors, we make use of the recently proposed BIP389 (https://github.com/bitcoin/bips/pull/1354). In order to prevent as much as possible introducing a backward incompatibility in the configuration file after the first release, we restrict the usage of multipath descriptors to `<0;1>` here.
  We now derive public keys from `xpub/0/*` and `xpub/1/*` while we were previously deriving them from `xpub/*`.

  This triggered a pretty invasive refactoring, as most parts of the codebase had to be updated to support the new change/receive separation (even the functional test miniscript dependency had to be updated, see https://github.com/darosior/python-bip380/pull/21).
  Broadly, this:
  1. Update our Miniscript dependency to my upstream PR (https://github.com/rust-bitcoin/rust-miniscript/pull/470) rebased on top of the 8.0.0 release.
  2. Updates the descriptors module to handle somewhat safely the multipath descriptors (to avoid mixing up the single, multi, and derived descriptors).
  3. Makes a multipath descriptor mandatory in the configuration file.
  4. Updates the Bitcoin backend poller aware of descriptors for which to track coins.
     - Necessarily this updates the bitcoind implementation to import two descriptors
  5. Record in database whether a coin was for the change or receive descriptor, in addition to its derivation index

ACKs for top commit:
  edouardparis:
    ACK 117171f24ff5bd6731d9e5e50e4515a03013a9eb

Tree-SHA512: efcb7267f1ba6a5a3072e96fd1c70272f81092e86ee1178833f83d0aa88f271f42c269b71ca9992e76bb3e103baf1a189a609cc20f14f29b7388ab133da99044
2022-10-28 13:44:29 +02:00
Antoine Poinsot
117171f24f
commands: use a separate key chain for change addresses 2022-10-24 15:00:17 +02:00
Antoine Poinsot
d9f905a19a
db: track the next unused derivation index for change, too 2022-10-24 15:00:17 +02:00
Antoine Poinsot
58a0e57c59
db: record whether a coin was received on a change address
So we know what descriptor to use when spending it.
2022-10-24 15:00:16 +02:00
Antoine Poinsot
9b04a55147
db: store derivation index also for addresses from the change desc
This doubles the storage required but there is no way around it if we
want the poller to detect those coins without grinding.
2022-10-24 15:00:16 +02:00
Antoine Poinsot
4f3daa7741
descriptors: cache the receive and change descriptors 2022-10-24 15:00:15 +02:00
Antoine Poinsot
ca3d7c1f33
descriptors: introduce a newtype for the multipath descriptor
The multipath descriptor has very different properties than the receive
and change ones. Use a newtype to assist us in differentiating those.
2022-10-24 15:00:15 +02:00
Antoine Poinsot
1320ee30ba
daemon: use multipath descriptors
In config, expect to be given a multipath descriptor that contains a
derivation path for both receive and change addresses, but only for
those.

Instead of 'xpub/*', start using 'xpub/0/*' and 'xpub/1/*'.

When creating the watchonly wallet on bitcoind import both the receive
and change descriptors.

When polling, check for coins on both descriptors.
2022-10-24 15:00:14 +02:00
Antoine Poinsot
d4db804e4b
qa: add a missing 'wait_for' in spend creation test 2022-10-24 15:00:14 +02:00
Antoine Poinsot
7a18c583cb
bitcoind: filter received coins based on parent descriptors
Our bitcoind watchonly wallet could, maybe, have other descriptors that
were imported. Sounds pretty unlikely since we use a dedicated wallet
but hey.

More importantly, we'll need to know the parent descriptor of the coin
in order to recognize it as newly received or change.
2022-10-24 15:00:13 +02:00
Antoine Poinsot
ba4c1e0383
bitcoind: include change outputs in listsinceblock 2022-10-24 15:00:13 +02:00
Antoine Poinsot
caaca1fd1a
descriptors: rename derive into derive_received
We'll be able to derive change addresses too
2022-10-24 15:00:12 +02:00
Antoine Poinsot
f985fd7879
descriptors: remove as_inner method
We'll change the semantic of the descriptor, so we need to make sure
nothing accesses it with the old semantic.
2022-10-24 15:00:12 +02:00
Antoine Poinsot
846d924792
qa: upgrade python-bip380 to latest master
For multipath descriptors support
2022-10-24 15:00:11 +02:00
Antoine Poinsot
3105b86a28
Use my own fork of rust-miniscript
It supports multipath descriptors. We'll need to find a solution for the release
2022-10-24 09:01:03 +02:00
edouard
a623bd0f7a
Merge #70: bump minisafe#790d28
813b9684a90b2cf9b39d6e96c9d383d8d32ba5b8 bump minisafe#790d28 (edouard)

Pull request description:

ACKs for top commit:
  edouardparis:
    ACK 813b9684a90b2cf9b39d6e96c9d383d8d32ba5b8

Tree-SHA512: 8af937fdeac2e6d7df77cfd57eb5ab047d7babdedfbe9db67304b066c3779bb61806da24b0b5cba7c18fd8ee1df51a6258ea8df72f8ad608ddad9711c07d10be
2022-10-21 11:39:41 +02:00
edouard
813b9684a9 bump minisafe#790d28 2022-10-21 10:43:08 +02:00
Antoine Poinsot
790d283e77
Merge #67: Update rust-miniscript to 8.0.0
49ccc28ca7b0ee6e266d79ace67ff35c3d50905c Cargo: update most dependencies to the latest version (Antoine Poinsot)
a4bdb1efb79b8b87ba6131dea96798766e1fbcf5 tree wide: upgrade to Miniscript 8.0.0 (Antoine Poinsot)
d432745da583e2c38e67cae540b9417d76867bfa Cargo: don't require the 'compiler' feature from miniscript (Antoine Poinsot)
f7ca86191a6972bb6a8e3d78261159648db4ca23 ci: temporary disable 1.48 build on macOS (Antoine Poinsot)
f3e93df80c5f400e0b8f83c4862f0ff4b9f0f367 qa: add missing sighash type to dummy signatures in test_update_spend (Antoine Poinsot)

Pull request description:

  Took the opportunity to update some dependencies, too.

ACKs for top commit:
  darosior:
    ACK 49ccc28ca7b0ee6e266d79ace67ff35c3d50905c

Tree-SHA512: babd0ac6af752e4671b1221f63bcc85cf9f1029162893c71ba991b2da0c16bf02e70bf061969b1a058d7c22d57061d99a82d4ed8564787a38f8a35a784d21812
2022-10-20 18:17:04 +02:00
Antoine Poinsot
49ccc28ca7
Cargo: update most dependencies to the latest version 2022-10-20 17:53:27 +02:00
Antoine Poinsot
a4bdb1efb7
tree wide: upgrade to Miniscript 8.0.0
This also updates the indirect rust-bitcoin dependency to 0.29.

Major changes are in the descriptors management:
	- The rust-miniscript descriptors don't support raw hashes
	  within the pk_h() fragments, so we don't need all the boutique
	  management for this.
	- The key translator API changed.
	- We now take a u16 for the timelock instead of our previous
	  checks. This was inspired by the new rust-bitcoin Sequence
	  type.
	- We now take a path instead of just a derivation index in
	  DerivedKey. We might use paths after all...

As for rust-bitcoin it's just a few nits:
	- No more 'global' field on PSBTs (yay)
	- Couple more trait derivation on types
	- Some APIs were renamed.
2022-10-20 17:53:26 +02:00
Antoine Poinsot
d432745da5
Cargo: don't require the 'compiler' feature from miniscript 2022-10-20 17:53:26 +02:00
Antoine Poinsot
f7ca86191a
ci: temporary disable 1.48 build on macOS 2022-10-20 17:53:25 +02:00
Antoine Poinsot
f3e93df80c
qa: add missing sighash type to dummy signatures in test_update_spend 2022-10-20 16:37:47 +02:00
Antoine Poinsot
9732ce8e29
Merge #65: A new broadcastspend command
b89401e5835b94f7e5341028357b11db69f8e37d qa: remove an unused variable in test_update_spend (Antoine Poinsot)
af9f0aeaed0f5607db94348c136325cf5fe9e387 qa: replace our PSBT implementation with a tweaked version of Bitcoin Core's (Antoine Poinsot)
eff39ee35a4b4d9133586ab5200689da40ef254f rpc: a new 'broadcastspend' command (Antoine Poinsot)
37ee93a1e6ddd5089c1cc3d2cfa6f263b691cf32 commands: a new command for broadcasting a Spend transaction (Antoine Poinsot)
b14bc602d45cac8191059d8ed6a5391ca7cfc68f bitcoin: interface for broadcasting a transaction (Antoine Poinsot)

Pull request description:

  This implements a new command to broadcast an existing Spend transaction.

  Fixes #58.
  Fixes #66.

ACKs for top commit:
  darosior:
    ACK b89401e5835b94f7e5341028357b11db69f8e37d

Tree-SHA512: 299f7ba1df48ff2bbda68055df885474f5ca2b8336c46403d5f0bdfc30ec66d52653615780c187c8cab23b756fd9dad97f02c7ac20c71d833c54183d3c2e5f0a
2022-10-19 15:55:54 +02:00
edouard
382962a47b
Merge #26: Add coins panel
b849f9bb3d8577f6a0c52e6903842ab72d602583 Update minisafe master (edouard)
86eddb28c625a96a0e9b3db7cc80dbb3eb0813f5 add coins panel to gui (edouard)

Pull request description:

  based on #22

ACKs for top commit:
  edouardparis:
    Self-ACK b849f9bb3d8577f6a0c52e6903842ab72d602583

Tree-SHA512: 9959ecd84594ce94c3c9ae383344615b533a1dc968268cb8a8336840750a9c6ee82ab29a7f45999e5d27ab79a296dd2396a0d9e56890f7063c38717e0f0f6391
2022-10-19 13:37:23 +02:00
edouard
b849f9bb3d Update minisafe master 2022-10-19 12:56:44 +02:00
edouard
86eddb28c6 add coins panel to gui 2022-10-19 12:56:44 +02:00
Antoine Poinsot
b89401e583
qa: remove an unused variable in test_update_spend
The new PSBT implementation brought it to light.
2022-10-19 12:07:22 +02:00
Antoine Poinsot
af9f0aeaed
qa: replace our PSBT implementation with a tweaked version of Bitcoin Core's
This replaces our existing implementation of PSBTs with a more
straightforward one, adapted from the Bitcoin Core functional tests
framework. This fixes a few flakes that occured because the previous
implementation could produce invalid PSBTs.

The Bitcoin Core implementation is pretty low level and was adapted to
treat mappings as such (the value in the PSBTMap can itself be a
mapping, like for partial signatures or BIP32 derivation paths).

The rest of the diff is adapting the users of PSBT to use the new
implementation and the clearly superior interface (yay!).
2022-10-19 12:07:22 +02:00
Antoine Poinsot
eff39ee35a
rpc: a new 'broadcastspend' command 2022-10-18 19:54:03 +02:00
Antoine Poinsot
37ee93a1e6
commands: a new command for broadcasting a Spend transaction 2022-10-18 19:48:37 +02:00
Antoine Poinsot
b14bc602d4
bitcoin: interface for broadcasting a transaction 2022-10-18 19:48:20 +02:00
Antoine Poinsot
9bb20303e7
Merge #63: Chain reorganization handling
e75637d3629df901b1aa453b131c1c780bfd8724 jsonrpc: fixup two typos in error messages (Antoine Poinsot)
a9b0e5e559b0ddddbd8e612771a95182f131e0f0 qa: functional tests for block chain reorganization (Antoine Poinsot)
e88bbbe65b34cf7af645fc3f07a90266d4a66792 poller: block chain reorganization handling (Antoine Poinsot)
d6f24e1c6a20d961128aa9c95367bd62aa8cb229 bitcoind: don't return spent coins with unfetchable spending tx as spent (Antoine Poinsot)
99ab0d7add5f9f3f924a6bd61263135c09e2a4cc commands: add a 'spend_info' field to the 'listcoins' entries (Antoine Poinsot)
57add1d86bb734b0f5291b2f266ef5652c93bc8d commands: return the DB's block height in 'getinfo' (Antoine Poinsot)
92f7ef12251309a30d100108ad82cd6790b5f4ac commands: make listcoins return all coins by default (Antoine Poinsot)
e9e4acd69de55e0c32c7e275e3b5318b5e161c46 db: database interface to rollback to a previous best block (Antoine Poinsot)
972c8dac86976723f4c417142999d4850c2fb9b6 db: require the spend block height from the DB interface (Antoine Poinsot)
6038843d33621c098d958d343c8b6d038922f72f database: rename coins' spent_at in spend_block_time (Antoine Poinsot)
cce227f80fbcbb32c5fca6fe78142131d7f35a53 bitcoin: interface to get the common block in our and the backend's chains (Antoine Poinsot)

Pull request description:

  This finally implements our reorganization handling. Like in revaultd, upon noticing a tip change that indicates a reorg happened in our Bitcoin backend we rollback our state to the common ancestor between our state and the new chain, then start rescanning from there. The logic is much more straightforward than in revaultd though, as there is no presigned transactions to care about.

  The PR grew a bit large as this needed a bit of preparatory work in order to be reasonably tested (and i noticed a few bugs and cleanups that slipped through review in #29). Please let me know if reviewers prefer that i split the prep work on the commands in another PR.

  Fixes #15.

ACKs for top commit:
  darosior:
    ACK e75637d3629df901b1aa453b131c1c780bfd8724

Tree-SHA512: 1ebb2a3e10b462b739e1d5cb831de946177436c8fad4dcb20eb575fd0f58bef98a86e25c5fe0ed07d946975f982a420940607a69e74f24a02ef16271c92eceba
2022-10-18 19:44:56 +02:00
Antoine Poinsot
e75637d362
jsonrpc: fixup two typos in error messages 2022-10-18 19:43:52 +02:00
Antoine Poinsot
a9b0e5e559
qa: functional tests for block chain reorganization 2022-10-18 19:15:55 +02:00
Antoine Poinsot
e88bbbe65b
poller: block chain reorganization handling 2022-10-18 19:15:54 +02:00
Antoine Poinsot
d6f24e1c6a
bitcoind: don't return spent coins with unfetchable spending tx as spent
It seems less bad to be able to check again at the next poll.
2022-10-18 19:15:54 +02:00
Antoine Poinsot
99ab0d7add
commands: add a 'spend_info' field to the 'listcoins' entries 2022-10-17 11:47:42 +02:00
Antoine Poinsot
57add1d86b
commands: return the DB's block height in 'getinfo'
It makes more sense than to return the Bitcoin backend's. And it's helpful to wait for sync in functional tests.
2022-10-17 11:47:41 +02:00
Antoine Poinsot
92f7ef1225
commands: make listcoins return all coins by default
We'll add some parameters to filter by status and/or outpoints later on.
But for now also list coins that were spent or are being spent.
2022-10-17 11:47:41 +02:00
Antoine Poinsot
e9e4acd69d
db: database interface to rollback to a previous best block 2022-10-17 11:47:40 +02:00
Antoine Poinsot
972c8dac86
db: require the spend block height from the DB interface
Hence add a 'spend_block_height' field to the 'coin' column in the
SQLite implementation. This also contains a couple cleanups, as well as
a fix (we were still checking if the blockheight was > 1).
2022-10-17 10:44:38 +02:00
Antoine Poinsot
6038843d33
database: rename coins' spent_at in spend_block_time 2022-10-17 10:27:03 +02:00
Antoine Poinsot
cce227f80f
bitcoin: interface to get the common block in our and the backend's chains
This will be used to get the common ancestor in the chain upon detecting
a block chain reorganization.
2022-10-17 10:26:23 +02:00
Antoine Poinsot
efe2cf634d
Merge #60: Clippification
7e911b8bb7ccca866db2afadb357615d3712a4cb ci: add Clippy check for both the daemon and the GUI (Antoine Poinsot)
6871407b1a2514c8c3b259a88cd5c267b206761e gui: clippification (Antoine Poinsot)
8b4866158bc2097a14857194d38bfb058678a55b daemon: clippification (Antoine Poinsot)

Pull request description:

  This fixes *all* clippy warning from the whole repository. Some of them are... Well very opinionated to say the least. But to avoid bikeshedding i just applied all of them. Let's start fresh!

  @edouardparis can you skim through the clippy fixes in the GUI folder? I checked the daemon one already.

  Fixes #10.

ACKs for top commit:
  edouardparis:
    ACK 7e911b8bb7ccca866db2afadb357615d3712a4cb

Tree-SHA512: acebc86425039511d090b58139e12c149463d7a288d0d2aed3ba75d303aa2034db65f39f7e00436d556e86aa382686acab6699e65afe10ea31238c0a48a0635c
2022-10-14 12:31:20 +02:00
Antoine Poinsot
7e911b8bb7
ci: add Clippy check for both the daemon and the GUI 2022-10-13 14:24:25 +02:00
Antoine Poinsot
6871407b1a
gui: clippification 2022-10-13 14:24:25 +02:00
Antoine Poinsot
8b4866158b
daemon: clippification
That's a minimal API break because of some error variants that were
Box'd.
2022-10-13 14:24:24 +02:00
Antoine Poinsot
2a3214460e
Merge #29: Check spend status in bitcoin poller
172cda19a0a72b77e3832f2e29ed7ff6f8062c44 bitcoin: avoid an unnecessary large clone() (Antoine Poinsot)
7513bcbf09f13f1625b989d52c52087048d90a73 bitcoind: use and_then instead of map().flatten() (Antoine Poinsot)
51f11a9e2f51945c7ec1f59a9acfd43d2f3977d3 looper: cleanup the check for spending coins' confirmation (Antoine Poinsot)
c9b6c6dedbc1e28b05543191238d0a81a92ba238 db: re-rename list_unspent_coins into unspent_coins (Antoine Poinsot)
3534e35b8721c5026b27d4d3c07b87dc6e4fa3dd bitcoin: remove erroneous block height check (Antoine Poinsot)
bb9897bdbb9926f6c3f8d9fb95339e45546372e2 Update coin txid if conflicting tx was confirmed (edouard)
3cf6bcbb98fc0034bc46ed58564e8aa4a1b6ffa6 add spent_coins to bitcoind poller (edouard)
94ee94edbdcdd917df131ad87893406f5f8597a2 Add blocktime and spent_at to coins table (edouard)

Pull request description:

  Add two new columns:

  - `blocktime`: timestamp of the block containing the transaction funding the coin.
  - `spent_at`: timestamp of the block containing the transaction spending the coin.

  Update the coin `spent_at` when the spend transaction is confirmed

ACKs for top commit:
  darosior:
    ACK 172cda19a0a72b77e3832f2e29ed7ff6f8062c44

Tree-SHA512: 4f12dd273784c1e8fab7f6427800fd10e6404d47e07e2293106d4454165dffb856cd65c5f4e4537867be403bd1790ce363968bfb94fec58ab02af3624ed68f22
2022-10-12 17:27:46 +02:00
Antoine Poinsot
172cda19a0
bitcoin: avoid an unnecessary large clone() 2022-10-12 17:25:56 +02:00