Introduce TxId and TxHash types
#90
Conversation
That is also an issue, but honestly the lack of type is the biggest issue we've regularly ran into when implementing dual funding / splicing, because we always forget that lightning messages always use the tx hash (whereas using a specific type would force us to think about which one we should use).
We could, I can add that! |
Done in b9b8397 |
We could add a test with 2 consecutive testnet blocks ? |
Done in 7680d1f, looks like this was indeed using the block hash. |
t-bast
force-pushed
the
clarify-txid-hash
branch
from
7680d1f to
3163708
Compare
|
Rebased to fix trivial conflict ( |
It was really footgunny to only use a `ByteVector32` here, as users could use a `TxHash` when a `TxId` was expected. Introducing explicit types ensures that functions can clearly specify which one they expect.
It was footgunny to use a `ByteVector32` here as well, as we can easily mistake one for the other.
t-bast
force-pushed
the
clarify-txid-hash
branch
from
3163708 to
567bb5d
Compare
|
Added to eclair in ACINQ/eclair#2742 and lightning-kmp in ACINQ/lightning-kmp#524 without any issue, it should be safe to move forward. |
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.
It was really footgunny to only use a
ByteVector32here, as users could use aTxHashwhen aTxIdwas expected. Introducing explicit types ensures that functions can clearly specify which one they expect.Codecs can then decide how to encode that on the wire: this will let us exclusively use
TxIdin lightning, and encode/decode it as aTxHashon the wire, which will greatly simplify our types. For example, we would definesplice_lockedas:And the codec would convert this to a
TxHashbefore encoding its bytes, since lightning wire messages always expect aTxHash. This way we hide that encoding detail in the codecs, and only deal withTxIds that we get fromTransactionobjects.