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

Use TxId and TxHash objects #524

Merged
merged 3 commits into from
Nov 28, 2023
Merged

Use TxId and TxHash objects #524

merged 3 commits into from
Nov 28, 2023

Conversation

t-bast
Copy link
Member

@t-bast t-bast commented Sep 1, 2023

This helps disambiguate when each is used. Even though confusingly, Electrum calls its field tx_hash while actually returning the txid.

This is mostly a trivial replacement from ByteVector32 to TxId and removing a lot of calls to reversed(). The only subtle parts are in the wire package, where we now only expose TxId but in most cases actually encode/decode a tx_hash.

@t-bast t-bast force-pushed the use-bitcoin-kmp-txid branch from d3f8384 to ae73360 Compare September 28, 2023 16:22
@t-bast t-bast marked this pull request as ready for review November 22, 2023 13:33
This helps disambiguate when each is used. Even though confusingly,
Electrum calls its field `tx_hash` while actually returning the `txid`.
@t-bast t-bast force-pushed the use-bitcoin-kmp-txid branch from ae73360 to 4235964 Compare November 23, 2023 09:50
@t-bast t-bast requested review from pm47 and sstone November 23, 2023 15:32
sstone
sstone previously approved these changes Nov 23, 2023
pm47
pm47 previously approved these changes Nov 28, 2023
Copy link
Member

@pm47 pm47 left a comment

Choose a reason for hiding this comment

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

LGTM

@t-bast t-bast dismissed stale reviews from pm47 and sstone via 4876605 November 28, 2023 14:37
@t-bast t-bast merged commit 2261535 into master Nov 28, 2023
2 checks passed
@t-bast t-bast deleted the use-bitcoin-kmp-txid branch November 28, 2023 14:51
dpad85 added a commit to ACINQ/phoenix that referenced this pull request Nov 30, 2023
See for details:
- ACINQ/bitcoin-kmp#90
- ACINQ/lightning-kmp#524

Transaction ids are still stored as byte arrays in the
database (use txId.value to get the raw data), but we
should use the correct types whenever possible.

Use toString() to get the txid as an hexa string.
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.

3 participants