-
Notifications
You must be signed in to change notification settings - Fork 58
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
Feat/lw 11589 make auxiliary data and witness available in sign request #1504
Feat/lw 11589 make auxiliary data and witness available in sign request #1504
Conversation
c26c773
to
271860c
Compare
|
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.
@mirceahasegan The finalize method only passes the body as CBOR because its the only thing it needs to sign the transaction properly (from CIP-30 endpoint). The transaction body has the hash of the original aux data and the script integrity hash, hashes the relevant parts of the witness set (that would alter the hash in the body), so its safe to pass the aux data and witness set in non CBOR, the caller of the sign transaction endpoint has the original aux data and witness set anyways, and we only return the signatures. When the wallet signs an internal transaction, the transaction is in Core types so it does reconstruct them but at that point is okay because there are not signatures yet.
But, were you having any particular issue? what problem is this PR trying to solve?
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.
LGTM
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.
Nice work!
Instead of only the transaction body. The witnesser request was missing the auxiliaryData and witness fields. Plus, it's more reliable to use the CBOR instead of reconstructing it.
84396c6
to
c18a551
Compare
Context
wallet.finalizeTx
has optional params forauxiliaryData, witnessSet and isValid
.cip30.signTx
did extract this data from the input transaction CBOR, to pass it down to thefinalizeTx
method.Further more, even if they are included, serializing the transaction body (passed as CBOR) with the
auxiliaryData and witnessSet
(as deserialised core types), might not result in the same CBOR, due to differences in serialization implementation.Proposed Solution
cip30.signTx always receives the entire transaction cbor, not just the transaction body.
Extend wallet.finalizeTx() to accept CBOR in the
tx
parameter. When provided, the other optional parameters (witnessSet, auxiliaryData, isValid) are ignored, and the transaction CBOR is passed as is to the witnesser.Important Changes Introduced