silent payments: integrate SPDK scanning and wallet IPC - #47
Conversation
|
|
||
| #[test] | ||
| #[allow(clippy::type_complexity)] | ||
| fn bip352_official_receiving_vectors() { |
There was a problem hiding this comment.
I think the word "official" can be dropped here. There is only one reference BIP
| std::fs::remove_file(&path).ok(); | ||
| } | ||
|
|
||
| fn hex_decode(s: &str) -> Vec<u8> { |
There was a problem hiding this comment.
Please remove this function in favor of the hex crate re-exported from bitcoin.
|
7bde2e0 includes a new file that isn't actually linked to the project. It should otherwise not compile. Let's start by separating the commits such that each commit compiles, and the |
|
I notice there is also bip 352 on crates.io. Any reason to prefer re-writing it instead of importing the existing crate, forking the existing crate, or forking and add a PR back to existing crate? If the existing bip352 crate is too unusable, maybe make a new crate for BIP 352? |
I would prefer to not have a dependency that is bitcoin-related that we don't directly manage the release cycle of or is not well-reviewed and widely used (i.e. BDK). Reviewing a copy-paste of that crate into here would be essentially the same amount of review. While I agree it's not great to have a complete re-write, I think we should go ahead and do it here, and once BDK has support for silent payments, we do a drop-in replacement. |
I'm not arguing against any or those points. However, I do think it would make sense for this to be contained within it's own crate. |
07022cc to
7e03e6d
Compare
|
Addressed all the feedback thus far in 7e03e6d Should we extract this into its own crate in this PR? |
I think so. |
|
I recommend something like |
|
There is also an entire "dev kit" for silent payments in Rust: https://github.com/cygnet3/spdk. I noticed this is being added to to electrs which is a mature project. |
|
Oh interesting... I trust both the lead dev and the electrs guys. We should probably look into this. |
|
I reviewed SPDK and have some thoughts about path forward. The examples look intuitive and easy to integrate. Although there are some drawbacks, in that the crate brings in quite a few dependencies, I have more faith in them (electrs) maintaining the crate than bringing in custom SP logic here. @pzafonte I think we should repurpose this PR to integrate SPDK, however there is a blocker before you can get started. I was overly optimistic on the timeline for |
|
#48 is merged so |
Great! I'll get started on this soon. |
aae03b7 to
a69aab9
Compare
Introduce crates/wallet as a workspace member containing a silentpayments submodule. The module wraps the silentpayments crate to provide scan_block, which takes kernel undo data (InputData) and returns found payments, and SilentPaymentWallet, which tracks owned UTXOs and history in memory. The crate is structured with pub mod silentpayments so other wallet backends can be added alongside it without restructuring.
Add wallet.capnp defining import_keys, get_balance, and get_history methods. Implement WalletIpcInterface and WalletState, which share the SilentPaymentWallet between the block-scanning thread and the IPC server via Arc<Mutex<>>. The wallet socket is kept separate from node.sock so wallet commands do not block node commands.
Wire scan_kernel_block into the block processing loop so each accepted block is scanned for payments to the imported keys. Read kernel undo data to recover prevout scripts and call wallet::silentpayments::scan_block. The scan is a no-op until keys are imported, so nodes without a wallet configured are unaffected. Start a dedicated wallet.sock listener alongside the existing node.sock so the IPC server and block-processing thread can share WalletState safely. Peers send a headers message rather than inv to announce new blocks to nodes that previously sent getheaders. Handle this in AwaitingInv so the node continues to receive new blocks.
Add import-keys, balance, and history subcommands under a wallet subcommand group. Each connects to wallet.sock rather than node.sock so wallet operations are independent of node RPC.
pzafonte
changed the title
Don't be afraid to open a new PR. This one is pretty scratched up with review comments that no longer apply. |
|
Opened fresh PR in #50 |
3 participants
Integrate BIP-352 silent payment scanning into kernel-node using the
silentpaymentscrate from SPDK.crates/walletas a workspace member with asilentpaymentssubmodule that wraps SPDK's cryptographic primitivesimport_keys,balance, andhistoryover a dedicated Cap'n Proto Unix socket (wallet.sock)