Skip to content
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

Support new Bulletproof rewind scheme #48

Merged
merged 7 commits into from
Jun 2, 2019

Conversation

jaspervdm
Copy link
Contributor

@jaspervdm jaspervdm commented May 26, 2019

This PR does a few things to enable support for mimblewimble/grin-wallet#105

  • Change the BP message from 16 to 20 bytes
  • Only check if first 4 bytes of mu are 0. For legacy proofs the first 4 bytes of the message should also be 0, this will be checked at the wallet level
  • Remove check of gamma*G + v*H == commit, this will be replaced by a re-derivation of the commitment using the recovered amount and message at the wallet level

There are accompanying changes in rust-libsecp and grin that need to be merged at the same time to not break backwards compatibility.

@jaspervdm jaspervdm changed the title Support new rewind method Support new Bulletproof rewind scheme May 27, 2019
@jaspervdm jaspervdm marked this pull request as ready for review May 27, 2019 21:27
@jaspervdm
Copy link
Contributor Author

This one is now ready for review. Requesting review from @yeastplume and @garyyu, but I welcome others as well :)

for (i=0; i<16; i++) {
vals_bytes[i+8] = message[i];
for (i=0; i<20; i++) {
vals_bytes[i+4] = message[i];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

vals_bytes[i+4]

here still should be [i+8]. why you want to lose the half of the value bytes?

Copy link
Contributor Author

@jaspervdm jaspervdm May 29, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the value is encoded in 24-31. See also in the rewind function

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, indeed, values are in 24-31 (i write a tiny test code to verify that).
Sorry for wrong alarm :-)

@yeastplume yeastplume merged commit 84563ed into mimblewimble:master Jun 2, 2019
@DavidBurkett
Copy link
Contributor

Bad news. Early versions of Grin++ had a bug where it would generate blinding factors a little differently than grin. The only reason you could restore old outputs when switching between the two was it would rewind the actual blinding factor, not just Keychain path. If we don't continue to use the old rewind method for old-style bulletproofs, early Grin++ outputs won't be restoreable in Grin. I can solve it with custom logic for those that continue to use Grin++, but I would like to be able to support it for a time in Grin, as well.

@DavidBurkett
Copy link
Contributor

DavidBurkett commented Jun 2, 2019

@jaspervdm pointed out that we are already ignoring the rewound blinding factor today, so this change does not make the problem worse.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants