Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backend: implement Silent Payments sending #284

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

webwarrior-ws
Copy link
Contributor

@webwarrior-ws webwarrior-ws commented Jul 15, 2024

Implemented sending of Silent Payments (BIP-352).

Created tests for Silent Payments based on test data for BIP-352.

Updated NBitcoin to version 7.0.13 because older NBitcoin
versions can't decode bech32 strings of length > 90, which is
needed for Silent Payment address decoding. Since
UnknowParameters property of BitcoinUrlBuilder is deprecated
in new version, use UnknownParameters instead. As return type
of new property is different, had to change some code.

@webwarrior-ws webwarrior-ws force-pushed the silent-payments-squashed branch 3 times, most recently from 14f35fe to 9d3c534 Compare July 15, 2024 12:44
@knocte
Copy link
Member

knocte commented Jul 16, 2024

With regards to the first commit message, I've noticed a pattern in the way you write them, when the commit does more than 1 thing. Let's say commit does A and B and C. You usually do this:

scope: this is some commit title

Do A because blah blah and blah (blah blah). Do B
given that blah blah blah. Do C as this is better than
blah.

Please stop using that style! If the commit contains more than one change, then do this:

scope: this is some commit title

Several things were achieved in this commit:
* A because blah blah and blah (blah blah).
* B given that blah blah blah.
* C as this is better than blah.

Or, if you prefer a prose style, then you have to link each thing A, B, C between them. For example:

scope: this is some commit title

Do A because blah blah and blah (blah blah). As a
result of this, B given that blah blah blah. Last but
not least, C as this is better than blah.

So please when listing things, either use bullets, or link each thing with conjunctions (especially if you can explain in which way A is related to B, how B is related to C or A, etc).

Updated NBitcoin to version 7.0.13 because older NBitcoin
versions can't decode bech32 strings of length > 90, which is
needed for Silent Payment address decoding. Since
UnknowParameters property of BitcoinUrlBuilder is deprecated
in new version, use UnknownParameters instead. As return type
of new property is different, had to change some code.
Implemented sending of Silent Payments (BIP-352).

Created tests for Silent Payments based on test data for
BIP-352.
Refactoring: addressed comments to the PR.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants