Skip to content

Introduce hash-like types#754

Merged
thunderbiscuit merged 1 commit intobitcoindevkit:masterfrom
rustaceanrob:hash-like-5-1
May 13, 2025
Merged

Introduce hash-like types#754
thunderbiscuit merged 1 commit intobitcoindevkit:masterfrom
rustaceanrob:hash-like-5-1

Conversation

@rustaceanrob
Copy link
Copy Markdown
Collaborator

@rustaceanrob rustaceanrob commented May 1, 2025

Types like "Txid", "BlockHash", "DescriptorId" are all just 32 byte arrays that represent hashes with different meanings. Currently they are represented as strings at the FFI layer, but they are also meaningful arrays of bytes. Particularly if a user wants to implement persistence over the FFI layer, they would want to efficiently serialize these types.

Here I am introducing a new group of types that all implement display, allow serialization to bytes, and may be constructed from an array of bytes. I went with a "rule of 3s" here, and also introduced a macro to do these implementations because there was a lot of boilerplate involved.

Note that all of these are included in the wallet changeset, which is required to represent in-full for FFI-layer custom persistence: https://docs.rs/bdk_wallet/latest/bdk_wallet/struct.ChangeSet.html

edit: Added TxMerkleNode, which may be interesting in the future if merkle inclusion proofs come to the bindings layer at some point for Electrum users.

edit: Added Wtxid, which is how transactions are identified over the p2p network as of SegWit. This is useful for Kyoto users.

Notes to the reviewers

  • I understand macros may not be the most legible, but this case in particular saves a lot of repeated logic.
  • "DescriptorId" from bdk_chain is mostly an alias for sha256::Hash

Changelog notice

  • Introduce object types for Txid, BlockHash, DescriptorId, Wtxid, TxMerkleNode

Checklists

All Submissions:

  • I've signed all my commits
  • I followed the contribution guidelines
  • I ran cargo fmt and cargo clippy before committing

@rustaceanrob rustaceanrob force-pushed the hash-like-5-1 branch 2 times, most recently from 9ee6616 to f77ab57 Compare May 1, 2025 18:59
@notmandatory
Copy link
Copy Markdown
Member

Concept ACK, and proc macros aren't too hard to read, especially simple ones like you added.

@rustaceanrob rustaceanrob force-pushed the hash-like-5-1 branch 6 times, most recently from 8537c18 to 161dc92 Compare May 7, 2025 12:04
@rustaceanrob
Copy link
Copy Markdown
Collaborator Author

A lot of my work on persistence relies on this change. @ItoroD can I get a review on this so I can respond to any outstanding feedback?

@ItoroD
Copy link
Copy Markdown
Collaborator

ItoroD commented May 8, 2025

A lot of my work on persistence relies on this change. @ItoroD can I get a review on this so I can respond to any outstanding feedback?

Sure! will take a look in a bit

Comment thread bdk-ffi/src/macros.rs
#[uniffi::constructor]
pub fn from_bytes(bytes: Vec<u8>) -> Result<Self, HashParseError> {
let hash_like: $core_type = deserialize(&bytes).map_err(|_| {
let len = bytes.len() as u32;
Copy link
Copy Markdown
Collaborator

@ItoroD ItoroD May 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried doing some serializations with this. One observation I have is, when we get a txid from a transaction, in kotlin transaction.computeTxid() we get a 64 char Hex. So I had to convert to a 32 bytes array before passing it in here. So I guess the idea is anyone looking to store will be storing them as bytes (32 bytes) That pretty standard.

The conversion does work well! I can approve if you have no other comments

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah in Kotlin this conversion can be done using the hexToByteArray() extension function. So you could do for example "abcd0123".hexToByteArray() to get your bytes.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally this type is used across the library, so you would be able to serialize to bytes instead of going through the string implementation

@ItoroD
Copy link
Copy Markdown
Collaborator

ItoroD commented May 8, 2025

ACK c533b0a

Copy link
Copy Markdown
Collaborator

@reez reez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK c533b0a

Types like "Txid", "Wtxid", "BlockHash", "DescriptorId", "TxMerkleNode" are all just 32 byte
arrays that represent hashes with different meanings. Currently they
are represented as strings at the FFI layer, but they are also
meaningful arrays of bytes. Particularly if a user wants to implement
persistence over the FFI layer, they would want to efficiently
serialize these types.

Here I am introducing a new group of types that all implement display,
allow serialization to bytes, and may be constructed from an array of
bytes. I went with a "rule of 3s" here, and also introduced a macro to
do these implementations because there was a lot of boilerplate
involved.

Note that all of these are included in the wallet changeset, which is
required to represent in-full for FFI-layer custom persistence.
@thunderbiscuit
Copy link
Copy Markdown
Member

Itoro's comment made me realize that a follow-up PR could use the Txid type across the codebase, including the Transaction::compute_txid() method.

I found a few more:

  • Transaction::compute_txid()
  • The txid field on the OutPoint type
  • The BumpFeeTxBuilder constructor
  • EsploraClient::get_tx
  • EsploraClient::get_tx_info
  • EsploraClient::get_tx_status
  • Wallet::get_tx
  • The txid field on the Tx type

Copy link
Copy Markdown
Member

@thunderbiscuit thunderbiscuit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 22b213c.

@thunderbiscuit thunderbiscuit merged commit 22b213c into bitcoindevkit:master May 13, 2025
26 checks passed
@rustaceanrob rustaceanrob deleted the hash-like-5-1 branch May 13, 2025 14:01
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.

5 participants