-
Notifications
You must be signed in to change notification settings - Fork 111
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
Create types for unmined transactions and their IDs #2634
Conversation
fca3cfb
to
a9f649f
Compare
d081e20
to
724c076
Compare
pub enum UnminedTxId { | ||
/// A narrow unmined transaction identifier. | ||
/// | ||
/// Used to uniquely identify transaction versions 1-4. | ||
Narrow(Hash), | ||
|
||
/// A wide unmined transaction identifier. | ||
/// | ||
/// Used to uniquely identify transaction version 5. | ||
Wide(WtxId), | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These TxId
s still apply post-mining though, right? They are not exclusively for un-mined transactions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For v1-v4 transactions, the transaction::Hash
is a unique identifier pre-mining (mempool and client) and post-mining (blockchain).
For v5 transactions, the WtxId
uniquely identifies a pre-mined transaction.
But post-mining, only the transaction::Hash
in their WtxId.id
is required for unique identification.
This is because v5 transactions have a non-malleable transaction::Hash
, which only covers the effects of the transaction (transparent and shielded spends and outputs). The authorizing data (proofs, signatures, and scripts) can change without changing the ID.
I've tried to update the documentation to clarify.
What do you think?
pub enum UnminedTxId { | |
/// A narrow unmined transaction identifier. | |
/// | |
/// Used to uniquely identify transaction versions 1-4. | |
Narrow(Hash), | |
/// A wide unmined transaction identifier. | |
/// | |
/// Used to uniquely identify transaction version 5. | |
Wide(WtxId), | |
} | |
pub enum UnminedTxId { | |
/// A narrow unmined transaction identifier. | |
/// | |
/// Used to uniquely identify unmined version 1-4 transactions. | |
/// (After v1-4 transactions are mined, they can be uniquely identified using the same [`transaction::Hash`].) | |
Narrow(Hash), | |
/// A wide unmined transaction identifier. | |
/// | |
/// Used to uniquely identify unmined version 5 transactions. | |
/// (After v5 transactions are mined, they can be uniquely identified using only their `WtxId.id`.) | |
/// | |
/// For more details, see [`WtxId`]. | |
Wide(WtxId), | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doc updates to clarify wide vs narrow transaction IDs.
pub enum UnminedTxId { | ||
/// A narrow unmined transaction identifier. | ||
/// | ||
/// Used to uniquely identify transaction versions 1-4. | ||
Narrow(Hash), | ||
|
||
/// A wide unmined transaction identifier. | ||
/// | ||
/// Used to uniquely identify transaction version 5. | ||
Wide(WtxId), | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For v1-v4 transactions, the transaction::Hash
is a unique identifier pre-mining (mempool and client) and post-mining (blockchain).
For v5 transactions, the WtxId
uniquely identifies a pre-mined transaction.
But post-mining, only the transaction::Hash
in their WtxId.id
is required for unique identification.
This is because v5 transactions have a non-malleable transaction::Hash
, which only covers the effects of the transaction (transparent and shielded spends and outputs). The authorizing data (proofs, signatures, and scripts) can change without changing the ID.
I've tried to update the documentation to clarify.
What do you think?
pub enum UnminedTxId { | |
/// A narrow unmined transaction identifier. | |
/// | |
/// Used to uniquely identify transaction versions 1-4. | |
Narrow(Hash), | |
/// A wide unmined transaction identifier. | |
/// | |
/// Used to uniquely identify transaction version 5. | |
Wide(WtxId), | |
} | |
pub enum UnminedTxId { | |
/// A narrow unmined transaction identifier. | |
/// | |
/// Used to uniquely identify unmined version 1-4 transactions. | |
/// (After v1-4 transactions are mined, they can be uniquely identified using the same [`transaction::Hash`].) | |
Narrow(Hash), | |
/// A wide unmined transaction identifier. | |
/// | |
/// Used to uniquely identify unmined version 5 transactions. | |
/// (After v5 transactions are mined, they can be uniquely identified using only their `WtxId.id`.) | |
/// | |
/// For more details, see [`WtxId`]. | |
Wide(WtxId), | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some comment clarifications about v5 transactions
Motivation
Zebra's mempool needs to handle unmined v4 and v5 transactions.
These transactions have different unique identifiers.
Specifications
https://zips.z.cash/protocol/protocol.pdf#txnidentifiers
https://zips.z.cash/zip-0239
https://zips.z.cash/zip-0244
Solution
UnminedTxId
enum, which holds a narrow or wide transaction ID, depending on versionUnminedTxId
UnminedTx
struct, which holds a transaction and its unmined IDReview
This PR blocks merging other mempool PRs that handle transaction IDs.
It's based on PR #2618, so it might need a rebase on
main
when that PR merges.Reviewer Checklist
We'll test this code when we use these types in
zebra-network
and inzebrad
's mempool.Follow Up Work
Use these types in internal protocol messages in
zebra-network
.This change is