test: verify the node receives a silent payment - #87
Conversation
|
Needs a rebase |
8126b30 to
b24a392
Compare
Is this just a temporary solution until core can send to a silent payment address? I see there are some PRs to add SilentPayments to core although I haven't inspected them closely. |
Silent payments sending definitely won't make it into the October release, so we're looking at 8-9 months minimum. The sending code is pretty readable here as well so I think it's fine. |
|
Tested 5 times and passed each time with no intermittent failures. Nice job. |
| const SYNC_TIMEOUT: Duration = Duration::from_secs(60); | ||
|
|
||
| #[test] | ||
| fn receives_a_silent_payment() { |
There was a problem hiding this comment.
I think it's a bad idea to add this as a unit test. This is by definition an integration test and should exist separately from unit tests.
|
Running IIUC I don't think running a bitcoin core node should be required to run the unit test suit. |
|
It isn't a unit test, integration tests run by default. |
|
That also fails for me.
|
|
I think it is maybe an annoyance at worst for us to have some extra steps for testing, but given Bitcoin Core is the dominant implementation and this is a node software, I think it is prudent we have tests for these scenarios. |
Core cannot send to a silent payment address, so the test builds the BIP-352 payment itself from a P2WPKH coinbase output, broadcasts it through Core, and mines it. The node, connected to Core, scans the block and its balance reflects the payment. Adds helpers to import keys and read the receive address and balance over the control socket, and silentpayments as a dev-dependency to build the payment.
b24a392 to
b25fdf1
Compare
corepc-node caps us at Core 30.2 anyway, so this would also need a silent-payments-capable Core to be available through corepc-node as well |
|
tACK b25fdf1 @yancyribbens if you have a different test layout suggestion please open an issue and I'll review |
|
Interesting, I didn't realize "cargo test" would run both unit and integration tests. I wonder if it would be worth documenting how to run wallet unit tests and also run unit tests separately in CI. I guess for now I can just "cd ./crates/wallet && cargo test" to run only the wallet unit tests. |
|
https://doc.rust-lang.org/cargo/commands/cargo-test.html#description:
The
|
Yeah I see that now, thanks.
This doesn't run any tests as you may have noticed. It doesn't look like the node has any unit tests which I think is the reason. |
|
Yeah, not sure why but needs the extra |
Oh, nice that works also with workspace: |
3 participants
Core cannot send to a silent payment address, so the test builds the BIP-352 payment itself from a P2WPKH coinbase output, broadcasts it through Core, and mines it. The node, connected to Core, scans the block and its balance reflects the payment.
Adds helpers to import keys and read the receive address and balance over the control socket, and silentpayments as a dev-dependency to build the payment.