citadel-tech / coinswap

Functioning, minimal-viable binaries and libraries to perform a trustless, p2p Maxwell-Belcher Coinswap Protocol
https://gist.github.com/chris-belcher/9144bd57a91c194e332fb5ca371d0964
Other
72 stars 46 forks source link

[improve] Use descriptor wallet in bitcoin core #119

Closed rajarshimaitra closed 6 months ago

rajarshimaitra commented 7 months ago

We are currently using legacy wallets in the bitcoin core rpc here https://github.com/utxo-teleport/teleport-transactions/blob/36eaaa956d22288e92da4df505bae725e1f037bc/src/wallet/rpc.rs#L93-L108

We are creating a legacy wallet for both the cases. Change that to descriptor wallet. This might cause changes in our address import. All addresses needs to be turned into raw descriptors before importing.

Descriptor wallet makes sync faster with rescanblockchain.

Jem256 commented 7 months ago

Hey @rajarshimaitra I would love to take this on

rajarshimaitra commented 7 months ago

Go ahead please..

Jem256 commented 6 months ago

Hey @rajarshimaitra I have created a draft PR with a few changes to the arguments such that a descriptor wallet is created. But I don't understand what you meant by "This might cause changes in our address import" I would like some clarification. when I run cargo test --features=integration-test --tests test_standard_coinswap -- --nocapture the test fails with Result::unwrap() on an Err value: Wallet(Rpc(JsonRpc(Rpc(RpcError { code: -4, message: "Only legacy wallets are supported by this command", data: None }))))`

rajarshimaitra commented 6 months ago

the test fails with Result::unwrap() on an Err value: Wallet(Rpc(JsonRpc(Rpc(RpcError { code: -4, message: "Only legacy wallets are supported by this command", data: None }))))`

Thats what I meant by changes in address import. If you look at the address importing code (check the line where this error is generating), its using something that is valid only for legacy wallet. That something has to be changed accordingly for descriptor wallet.