Using rust-bitcoin lib for Bitcoin primitives

[nlanson]

Any thoughts on using the rust-bitcoin library for Bitcoin structures in this library?

Objects like script, outpoints and tx hashes are well implemented in rust-bitcoin and would improve code readability and integration with other projects that use rust-bitcoin.

And if the plan is to eventually bring this project under the rust-bitcoin org it would make that process more streamlined too.

[cygnet3]

Yes this is definitely useful. It is related to #43 (I shortly mention it there in the last line). My plan was to add this as an optional feature.

The reason it is not (yet) part of this library is because this library was initially intended to be a very low-level cryptography library. Things like parsing transactions (e.g. getting the outpoints hash from a given transaction) are already too high-level and should be handled before calling this library.

But now I'm thinking that approach is not desirable. This means you're outsourcing a lot of the logic that is required for doing silent payments. It is why the tests/common/utils.rs file contains a bunch of helper functions that the client/wallet is pretty much forced to copy.

So instead this library will probably add a bunch of higher-level utility functions as optional features. One of those will be for rust-bitcoin, but I'm still thinking about how to structure it properly.

[cygnet3]

Another issue that I will comment on here: I'm wondering how to deal with versioning issues when adding rust-bitcoin as a dependency.

For example, the latest version of secp256k1 is 0.27, but BDK is using version 0.24. So if this library were to have 0.27 as a dependency, you would not be able to use the same PublicKey and SecretKey structs from BDK and provide them as arguments for functions from this library.

This is the reason why this library uses secp256k1 0.24: to be compatible with BDK. I'm sure that if we also add rust-bitcoin as a dependency, we get more issues like this.

If you have a good way of handling these kinds of versioning issues, then let me know.

[nlanson]

I am in favour of this library providing almost e2e support for sending and receiving silent payments (ie all the way from outpoints hashing and DH key calculation to address encoding + scanning (I still need to read up on scanning)). It doesn't make much sense otherwise, I'd argue getting users to do their own secrets computation is unsafe.

Re: versioning Downgrade everything to be compatible. rust-bitcoin reexports secp too.

On Thu, 21 Sept 2023, 03:05 cygnet, @.***> wrote:

Another issue that I will comment on here: I'm wondering how to deal with versioning issues when adding rust-bitcoin as a dependency.

For example, the latest version of secp256k1 is 0.27, but BDK is using version 0.24. So if this library were to have 0.27 as a dependency, you would not be able to use the same PublicKey and SecretKey structs from BDK and provide them as arguments for functions from this library.

This is the reason why this library uses secp256k1 0.24: to be compatible with BDK. I'm sure that if we also add rust-bitcoin as a dependency, we get more issues like this.

If you have a good way of handling these kinds of versioning issues, then let me know.

— Reply to this email directly, view it on GitHub https://github.com/cygnet3/rust-silentpayments/issues/53#issuecomment-1728126538, or unsubscribe https://github.com/notifications/unsubscribe-auth/AIGL7VH6GB36XRN3KPGEN2LX3MO7NANCNFSM6AAAAAA47EKUHU . You are receiving this because you authored the thread.Message ID: @.***>

[vincenzopalazzo]

rust-bitcoin reexports secp too.

Correct but secp is less common that will upgrade I think, but yeah there is this risk.

There are two option here:

1. Choose a version of bitcoin core, but this can cause version issue;
2. Works with bites, but this required that library like secp and rust-bitcoin expose nice API from and to bytes convertions

[nlanson]

No FFI interfacing is done with core so why would you need to match versions with core?

RB does provide traits for byte conversion, but there is no need to work with raw bytes (except for byte concatenation). Other logic is implemented through RB and secp (which RB reexports a compatible version of).

I'll try crop up a demo when I have some time to try and get my idea across more concretely.

On Thu, 21 Sept 2023, 22:09 Vincenzo Palazzo, @.***> wrote:

rust-bitcoin reexports secp too.

Correct but secp is less common that will upgrade I think, but yeah there is this risk.

There are two option here:

1. Choose a version of bitcoin core, but this can cause version issue;
2. Works with bites, but this required that library like secp and rust-bitcoin expose nice API from and to bytes convertions

— Reply to this email directly, view it on GitHub https://github.com/cygnet3/rust-silentpayments/issues/53#issuecomment-1729441186, or unsubscribe https://github.com/notifications/unsubscribe-auth/AIGL7VHK7IQIYZYNHDB73TDX3QU7JANCNFSM6AAAAAA47EKUHU . You are receiving this because you authored the thread.Message ID: @.***>

[vincenzopalazzo]

I do not understand what is core in your context.

If you are exporting a type from library A version x, the caller should use the library A with version X too otherwise there is a type mismatch from the compiler

[nlanson]

1. Choose a version of bitcoin core, but this can cause version issue;

core

If you are exporting a type from library A version x, the caller should use the library A with version X

For example, we use rust-bitcoin 0.30.1 (latest release on crates.io), then we can use the version of secp exported by rust-bitcoin through the use of pub use (or equivalent) which would be compatible with relevant rust-bitcoin's version of secp.

In the case of this lib, @cygnet3 seems to want the crate version to be compatible with bdk (I think its better to be compatible with the latest release of rust-bitcoin since bdk depends on rust-bitcoin and I want to believe this crate can eventually be merged as a module under the bitcoin crate if we try, but that is a discussion for another time when the BIP gets approved) so we can use the version of rust-bitcoin that latest bdk release uses which will have a reexport of the secp version that bdk uses.

However, while this crate is lone standing, it may be best to simply use the bdk compatible version of rust-bitcoin and reexport secp and other necessary stuff for users to avoid versioning issues on the user side.

Again, I'll try whip up a demo on the weekend...

[vincenzopalazzo]

Choose a version of bitcoin core, but this can cause version issue;

My fault, bitcoin core here do not make sense, I want to write rust bitcoin.

For example, we use rust-bitcoin 0.30.1 (latest release on crates.io), then we can use the version of secp exported by rust-bitcoin through the use of pub use (or equivalent) which would be compatible with relevant rust-bitcoin's version of secp.

This does not work in any case. But this is just my point of view, maybe I am just biased because a lot of my application was broken by rust-bitcoin .30

However, while this crate is lone standing, it may be best to simply use the bdk compatible version of rust-bitcoin and reexport secp and other necessary stuff for users to avoid versioning issues on the user side.

This create a problem for a not-wallet application like nakamoto. At the current state of the art we do not know how much work is required to make the library bdk compatible,or maybe you can do in your PoC

[nlanson]

This does not work in any case. But this is just my point of view, maybe I am just biased because a lot of my application was broken by rust-bitcoin .30

Isn't that where apps make a decision on whether to update the dependency by fixing breaking changes or continuing to use a lower version?

This create a problem for a not-wallet application like nakamoto. At the current state of the art we do not know how much work is required to make the library bdk compatible,or maybe you can do in your PoC

Again this is why this crate should keep in line with the latest rust-bitcoin version (and then be integrated into rust-bitcoin later down the line) instead of worrying about being compatible with other libraries like BDK, especially when BDK itself depends on rust-bitcoin.

[vincenzopalazzo]

I do not think that this library has anything to do with rust-bitcoin, but again whatever for me it is fine. The ecosystem is already full of this practice, so we will not solve the problem on this issue.

Again this is why this crate should keep in line with the latest rust-bitcoin version

I can not update to rust-bitcoin 0.30 due missing of feature, so I should take time to implement also these features in rust-bitcoin before upgrading

[benma]

I am strongly in favor of this.

Regardless, String as a main type has the downsides that the compiler cannot catch a large class of mistakes, and that it's not self-documenting.

Examples:

https://docs.rs/silentpayments/0.2.0/silentpayments/utils/sending/fn.calculate_partial_secret.html uses String for the transaction IDs, but a concrete type for it would be much better, for example https://docs.rs/bitcoin/latest/bitcoin/struct.OutPoint.html as the type for the outpoint.

https://docs.rs/silentpayments/0.2.0/silentpayments/sending/fn.generate_recipient_pubkeys.html uses String for the address, but an address type would be safer and easier to use (SilentPaymentAddress already exists and comes with TryFrom, so why not use that?).

If bitcoin as a dep is not desirable for some reason, then adding internal types for these would still give big benefits. In this case, the From trait could be implemented to convert from the bitcoin types, guarded by a feature.

[cygnet3]

I agree that Strings are not the best approach. There is some concern with adding a big dependency like rust-bitcoin. We had an issue with nakamoto which uses its own (outdated) version of rust-bitcoin in its api, and this caused all kinds of annoying version mismatch issues. That's why we opted to stick with using primitive types for now.

Of course, if enough people want this, then that could change. I think for now, using the internal type approach is probably a good choice.