TxSummary digest by cbeck88 · Pull Request #52 · mobilecoinfoundation/mcips

cbeck88 · 2022-10-10T01:27:32Z

A proposal to introduce a TxSummary object and incorporate it into the digest which MLSAG's sign, to help hardware wallets.

Rendered Proposal

eranrund

This looks reasonable to me, thank you for the detailed write up.
Do you think it would make sense to add a "worst possible case" calculation of how much memory the hardware wallet will need to support, based on a transaction that has the max inputs/outputs?

text/0052-tx-summary-digest.md

cbeck88 · 2022-10-11T15:20:50Z

Do you think it would make sense to add a "worst possible case" calculation of how much memory the hardware wallet will need to support, based on a transaction that has the max inputs/outputs?

Koe made a different estimate of the sizes of Tx's due to membership proofs which is larger than mine, in MCIP #21

Currently (protocol v0), Merkle membership proofs constitute the bulk of transaction size. A 1-input/1-output transaction is ~21 kB, with membership proofs taking up ~17 kB (assuming membership proofs with 32 elements, representing a Merkle tree 31 layers deep). In a 16-input/16-output transaction (the maximum size of a transaction), membership proofs occupy ~275 kB out of ~325 kB.

I have not actually done this calculation myself.

We could also write a unit test or something that measures this maximum size.

You're right, we should probably document what the actual worst possible case is at the current version (v2)

eranrund · 2022-10-11T15:29:09Z

Do you think it would make sense to add a "worst possible case" calculation of how much memory the hardware wallet will need to support, based on a transaction that has the max inputs/outputs?

Koe made a different estimate of the sizes of Tx's due to membership proofs which is larger than mine, in MCIP #21

Currently (protocol v0), Merkle membership proofs constitute the bulk of transaction size. A 1-input/1-output transaction is ~21 kB, with membership proofs taking up ~17 kB (assuming membership proofs with 32 elements, representing a Merkle tree 31 layers deep). In a 16-input/16-output transaction (the maximum size of a transaction), membership proofs occupy ~275 kB out of ~325 kB.

I have not actually done this calculation myself.

We could also write a unit test or something that measures this maximum size.

You're right, we should probably document what the actual worst possible case is at the current version (v2)

I think it's also valuable (if not more valuable) to document how much it would be following this change. This will make it easier for potential hardware integrations to have a quick go/no-go based on memory requirements.

remoun

LGTM

text/0052-tx-summary-digest.md

@remoun

Incorporate several of @remoun's and @nick-mobilecoin's suggestions. Co-authored-by: Remoun Metyas <remoun.metyas@gmail.com> Co-authored-by: Nick Santana <nick@mobilecoin.com>

isis-mc

LGTM

text/0052-tx-summary-digest.md

isis-mc · 2022-10-17T18:34:22Z

text/0052-tx-summary-digest.md

+The verifier similarly computes this
+`extended-message-and-tx-summary digest` and verifies that the MLSAGs sign this.
+
+When a hardware wallet is asked to sign an MLSAG, we can give it now the `extended_message_digest`
+and the `TxSummary`, and it can compute the appropriate digest from this for the MLSAG to sign.


This means we'll need a merlin implementation that runs on embedded devices, correct?

As a side note, I don't think we'll ever need to worry about running into a big-endian embedded device, but there's currently an endianness bug in our limited STROBE implementation used in merlin, which we'd want to fix before porting to C.

does the rust merlin implementation not work? can we just fix that one?

text/0052-tx-summary-digest.md

Co-authored-by: isis lovecruft <79881025+isis-mc@users.noreply.github.com>

Co-authored-by: Nick Santana <nick@mobilecoin.com>

Co-authored-by: isis lovecruft <79881025+isis-mc@users.noreply.github.com>

text/0052-tx-summary-digest.md

eranrund

This is really well written. Thank you!

text/0052-tx-summary-digest.md

Co-authored-by: Eran Rundstein <eran@rundste.in>

cbeck88 · 2022-11-03T19:13:11Z

Going to hold this one an extra day due to comments

add tx_summary_digest proposal

eac8232

cbeck88 requested review from a team, briancorbin, eranrund, isis-mc, jcape, nick-mobilecoin, remoun and sugargoat October 10, 2022 01:27

fixup

d97018a

cbeck88 changed the title ~~add tx_summary_digest proposal~~ TxSummary digest Oct 10, 2022

This was referenced Oct 10, 2022

Tracking issue for MCIP 52 POC mobilecoinfoundation/mobilecoin#2686

Closed

Add TxSummary object and incorporate it into MLSAG signing digest mobilecoinfoundation/mobilecoin#2683

Merged

fixups

af8740f

eranrund reviewed Oct 10, 2022

View reviewed changes

text/0052-tx-summary-digest.md Outdated Show resolved Hide resolved

cbeck88 added 7 commits October 11, 2022 10:08

fixups, more diagrams

ccad216

fixup

797f043

fixup

ded6b37

fixup mermaid

d74895d

restructure the reference level part

46a91bf

improve text

3a4b227

improve mermaid annotations

0e4bb48

remoun approved these changes Oct 14, 2022

View reviewed changes

nick-mobilecoin approved these changes Oct 14, 2022

View reviewed changes

Apply suggestions from code review

f1f67e5

Incorporate several of @remoun's and @nick-mobilecoin's suggestions. Co-authored-by: Remoun Metyas <remoun.metyas@gmail.com> Co-authored-by: Nick Santana <nick@mobilecoin.com>

isis-mc approved these changes Oct 17, 2022

View reviewed changes

Chris Beck and others added 2 commits October 18, 2022 11:04

Update text/0052-tx-summary-digest.md

1817daf

Co-authored-by: isis lovecruft <79881025+isis-mc@users.noreply.github.com>

rewrite guide-level explanation

2060f3f

Chris Beck and others added 13 commits October 18, 2022 12:11

Update text/0052-tx-summary-digest.md

3c862bd

Co-authored-by: isis lovecruft <79881025+isis-mc@users.noreply.github.com>

Update text/0052-tx-summary-digest.md

6f870fc

Co-authored-by: isis lovecruft <79881025+isis-mc@users.noreply.github.com>

Update text/0052-tx-summary-digest.md

d8ba50b

Co-authored-by: Nick Santana <nick@mobilecoin.com>

Update text/0052-tx-summary-digest.md

abce097

Co-authored-by: isis lovecruft <79881025+isis-mc@users.noreply.github.com>

Update text/0052-tx-summary-digest.md

a603a99

Co-authored-by: isis lovecruft <79881025+isis-mc@users.noreply.github.com>

fixups, add discussion about chain of trust

3d26611

fixup formatting

9b6f6d5

fix typos

379f114

fixup language

4753259

address isis comment

9316d54

add a disclaimer

22dd341

add clarifying text

7928a22

update with discussion about inputs with rules, rearrange slightly

f56ca8e

cbeck88 added the final-comment-period This RFC is in the final-comment period for the next 24h label Oct 21, 2022

cbeck88 commented Oct 31, 2022

View reviewed changes

text/0052-tx-summary-digest.md Outdated Show resolved Hide resolved

update with input rules digest

3d14da9

eranrund approved these changes Nov 3, 2022

View reviewed changes

cbeck88 and others added 6 commits November 3, 2022 10:18

add a note about the input rules digest

08ea9f6

Update text/0052-tx-summary-digest.md

708b897

Co-authored-by: Eran Rundstein <eran@rundste.in>

Update text/0052-tx-summary-digest.md

6767ba3

Co-authored-by: Eran Rundstein <eran@rundste.in>

Update text/0052-tx-summary-digest.md

400ebf2

Co-authored-by: Eran Rundstein <eran@rundste.in>

Update text/0052-tx-summary-digest.md

5462771

Co-authored-by: Eran Rundstein <eran@rundste.in>

add note about signing oracle

5c82be6

cbeck88 merged commit 788c995 into mobilecoinfoundation:main Nov 7, 2022

Conversation

cbeck88 commented Oct 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eranrund left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cbeck88 commented Oct 11, 2022

Uh oh!

eranrund commented Oct 11, 2022

Uh oh!

remoun left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

isis-mc left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

isis-mc Oct 17, 2022

Choose a reason for hiding this comment

Uh oh!

cbeck88 Oct 18, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

eranrund left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cbeck88 commented Nov 3, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

cbeck88 commented Oct 10, 2022 •

edited

Loading