-
Notifications
You must be signed in to change notification settings - Fork 4
Add PLACEHOLDER_ISSUANCE_VALUE to simplicity-core
#53
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
Conversation
8d7c5f4 to
4a64a6b
Compare
simplicity-core
| /// `PLACEHOLDER_ISSUANCE_VALUE` exists because an issuance token doesn't require | ||
| /// to have a unique `value` field for blinding. | ||
| /// For token issuance, the exact value inserted is irrelevant. | ||
| pub const PLACEHOLDER_ISSUANCE_VALUE: u64 = 0; |
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.
Could you explain more?
Currently it looks like a Magic value
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.
The idea is to use fixed value instead of any value, which is derived from utxos.
Yeah, in some kind this value is used for blinding, but it also can be executed with PLACEHOLDER_ISSUANCE_VALUE.
In further code on transaction analysis value field is unused by now, so I wished to replace it with constant.
options/creation_option.rs
basic/issue_asset.rs
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.
In comment for this constant we should mention this example, and add reasoning why it can be done, and mention when the actual values should be in TxOutSecrets
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.
The main information is in the comment. The reissuance and issuance tokens have to be defined and used in the order they were created, not the other way around.
https://github.com/ElementsProject/rust-elements/blob/2d94f5dcb5e4a81838681c71ed8a49b337feecd2/src/blind.rs#L652
How the blinding happens:
- https://github.com/ElementsProject/rust-elements/blob/2d94f5dcb5e4a81838681c71ed8a49b337feecd2/src/pset/mod.rs#L479
Blind last - the function blinds all inputs and commitments to the last output. - https://github.com/ElementsProject/rust-elements/blob/2d94f5dcb5e4a81838681c71ed8a49b337feecd2/src/pset/mod.rs#L493
Inside this function, we can see that first, we gather SurjectionProofs and then blind the outputs.
Gathering surjection inputs has a condition related to the issuance token. We can see that onlyasset_idis copied and the value is set to 0.
// This value really does not matter in surjection proofs
https://github.com/ElementsProject/rust-elements/blob/2d94f5dcb5e4a81838681c71ed8a49b337feecd2/src/pset/mod.rs#L438
So, after that, we can say with confidence that the value can be omitted and set to PLACEHOLDER_ISSUANCE_VALUE.
908a7c7 to
79fd8fd
Compare
79fd8fd to
1b51de6
Compare
KyrylR
left a comment
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.
ACK 1b51de6
Add a constant, as the
valuefield is optional for issuance tokens.The actual value is irrelevant for further transaction analysis and can be omitted.