Add TxSummary object and incorporate it into MLSAG signing digest #2683
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cbeck88
requested review from
a team,
briancorbin,
eranrund,
isis-mc,
jcape,
remoun and
ryankurte
cbeck88
added
enhancement
4 tasks
Contributor
Author
|
There is now an MCIP for this, we should bring this code into alignment with the MCIP spec: mobilecoinfoundation/mcips#52 |
cbeck88
added
the
transaction-core
remoun
reviewed
Oct 13, 2022
The motivation here is that, hardware wallets like Ledger have an expectation that the device can verify what is the balance change that will result from signing a given transaction and who the outbound money will be paid to. However, currently we have scoped things so that hardware wallets just are responsible for signing the Ring MLSAG, and they don't see large parts of the Tx, including the TxPrefix. The only way that the set of outputs is connected to the RingMLSAG is that the TxPrefix is hashed into the "message", which is hashed with more things to produce the "extended_message_digest", which is the 32 byte digest that the RingMLSAG actually signs. For the hardware wallet to really know what it is signing then, it would have to reproduce the entire extended message digest. The problem with this is that the TxPrefix can be > 100kb, which is much larger than the few kb that the hardware device currently has to handle, and is much too big to fit in the device's memory. Instead, we introduce a `TxSummary` object and add an extra step where it is hashed in just at the very end. The computer can then prove that the hash the MLSAG is signing is connected to a given TxSummary, and provide enough information from there to understand the balance delta and where the money is going. This manages to avoid sending all the merkle proofs for all the mixins and so it can be done sending only a few kb. The hope is to do this for v3, otherwise this issue may become the long poll for shipping hardware wallet support.
Co-authored-by: Remoun Metyas <remoun@mobilecoin.com>
Co-authored-by: Remoun Metyas <remoun@mobilecoin.com>
Co-authored-by: Remoun Metyas <remoun@mobilecoin.com>
Co-authored-by: Remoun Metyas <remoun@mobilecoin.com>
Co-authored-by: Remoun Metyas <remoun@mobilecoin.com>
Co-authored-by: Remoun Metyas <remoun@mobilecoin.com>
cbeck88
force-pushed
the
feature/tx-summary-digest
branch
from
dea1851 to
c622dbb
Compare
Contributor
Author
|
Still to do:
I am thinking to do the unblinding data in a separate PR staged on this one to ease review |
cbeck88
commented
Oct 17, 2022
nick-mobilecoin
approved these changes
Oct 17, 2022
Co-authored-by: Nick Santana <nick@mobilecoin.com>
cbeck88
commented
Oct 21, 2022
isis-mc
approved these changes
Nov 1, 2022
Contributor
isis-mc
left a comment
isis-mc
left a comment
There was a problem hiding this comment.
LGTM, small nits detailed above.
dolanbernard
pushed a commit
to dolanbernard/mobilecoin
that referenced
this pull request
Nov 2, 2022
…bilecoinfoundation#2683) * Add TxSummary object and incorporate it into MLSAG signing digest The motivation here is that, hardware wallets like Ledger have an expectation that the device can verify what is the balance change that will result from signing a given transaction and who the outbound money will be paid to. However, currently we have scoped things so that hardware wallets just are responsible for signing the Ring MLSAG, and they don't see large parts of the Tx, including the TxPrefix. The only way that the set of outputs is connected to the RingMLSAG is that the TxPrefix is hashed into the "message", which is hashed with more things to produce the "extended_message_digest", which is the 32 byte digest that the RingMLSAG actually signs. For the hardware wallet to really know what it is signing then, it would have to reproduce the entire extended message digest. The problem with this is that the TxPrefix can be > 100kb, which is much larger than the few kb that the hardware device currently has to handle, and is much too big to fit in the device's memory. Instead, we introduce a `TxSummary` object and add an extra step where it is hashed in just at the very end. The computer can then prove that the hash the MLSAG is signing is connected to a given TxSummary, and provide enough information from there to understand the balance delta and where the money is going. This manages to avoid sending all the merkle proofs for all the mixins and so it can be done sending only a few kb. The hope is to do this for v3, otherwise this issue may become the long poll for shipping hardware wallet support. * store input_rules with TxSummary, bring TxSummary in line with spec * fixup * Update transaction/core/src/tx_summary.rs Co-authored-by: Remoun Metyas <remoun@mobilecoin.com> * Update transaction/types/src/block_version.rs Co-authored-by: Remoun Metyas <remoun@mobilecoin.com> * Update transaction/core/src/tx_summary.rs Co-authored-by: Remoun Metyas <remoun@mobilecoin.com> * Update transaction/core/src/tx_summary.rs Co-authored-by: Remoun Metyas <remoun@mobilecoin.com> * Update transaction/core/src/tx_summary.rs Co-authored-by: Remoun Metyas <remoun@mobilecoin.com> * Update transaction/core/src/tx_summary.rs Co-authored-by: Remoun Metyas <remoun@mobilecoin.com> * fixups to idea for SCI support in TxSummary * Update transaction/core/src/tx_summary.rs Co-authored-by: Nick Santana <nick@mobilecoin.com> * lint and clippy * fixup TxOutSummary tags * review comments, add types for digests, move digests to new module * fixups to "associated_to_input_rules" test for TxOutSummary * improved code comments and cargo fmt Co-authored-by: Remoun Metyas <remoun@mobilecoin.com> Co-authored-by: Nick Santana <nick@mobilecoin.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The motivation here is that, hardware wallets like Ledger have an expectation that the device can verify what is the balance change that will result from signing a given transaction and who the outbound money will be paid to.
However, currently we have scoped things so that hardware wallets just are responsible for signing the Ring MLSAG, and they don't see large parts of the Tx, including the TxPrefix.
The only way that the set of outputs is connected to the RingMLSAG is that the TxPrefix is hashed into the "message", which is hashed with more things to produce the "extended_message_digest", which is the 32 byte digest that the RingMLSAG actually signs.
For the hardware wallet to really know what it is signing then, it would have to reproduce the entire extended message digest. The problem with this is that the TxPrefix can be > 100kb, which is much larger than the few kb that the hardware device currently has to handle, and is much too big to fit in the device's memory.
Instead, we introduce a
TxSummaryobject and add an extra step where it is hashed in just at the very end. The computer can then prove that the hash the MLSAG is signing is connected to a given TxSummary, and provide enough information from there to understand the balance delta and where the money is going. This manages to avoid sending all the merkle proofs for all the mixins and so it can be done sending only a few kb.The hope is to do this for v3, otherwise this issue may become the long pole for shipping hardware wallet support.
For other clients and servers, this doesn't meaningfully affect the time to sign or verify a Tx.