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

[WIP] Aggsig Transactions #530

Merged
merged 18 commits into from Jan 10, 2018

Conversation

yeastplume
Copy link
Member

@yeastplume yeastplume commented Dec 21, 2017

See #399 for context

https://github.com/mimblewimble/rust-secp256k1-zkp/blob/master/src/aggsig.rs for latest state of rust aggsig API (still very subject to change)

  • Integrate existing aggsig work into mimblewimble secp fork
  • creation of single-sig version of API based on existing work
  • Additions to single-sig API to support encoding different nonces within schnorr 'e' message (to support @tromp's additive signature scheme)
  • Additions to single-sig API to support simple addition of two signatures
  • (FIXED) The implementation I have doesn't verify the individual signatures AND the aggregated signature consistently, at the moment I can consistently validate one or the other, but not both. They should both work, and I'm currently working through the maths to figure out what the issue is.
  • (Framework done) Automated testing to exercise this (at the moment, there are no tests in the wallet whatsover.. integration tests need to be added, probably a separate file) in grin/tests
  • Updating transaction workflow between client and receiver as needed, with approach to keeping state between calls
  • Updating validation
  • Updating all automated tests

Note that stdout transactions are gone (as we don't have a strategy to handle them now that this is an exchange instead of a single send)

Also, I've added an optional config parameter to turn off waiting for peers on startup (which was slowing down automated testing immensely)

@yeastplume yeastplume added this to the Testnet2 milestone Dec 21, 2017
@yeastplume
Copy link
Member Author

Updated with progress and more detailed tasks.

@codecov-io
Copy link

codecov-io commented Jan 2, 2018

Codecov Report

❗ No coverage uploaded for pull request base (master@6a9a584). Click here to learn what that means.
The diff coverage is 0%.

Impacted file tree graph

@@           Coverage Diff           @@
##             master   #530   +/-   ##
=======================================
  Coverage          ?     0%           
=======================================
  Files             ?     33           
  Lines             ?   2922           
  Branches          ?      0           
=======================================
  Hits              ?      0           
  Misses            ?   2922           
  Partials          ?      0
Impacted Files Coverage Δ
core/src/core/transaction.rs 0% <ø> (ø)
keychain/src/keychain.rs 0% <0%> (ø)
util/src/lib.rs 0% <0%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6a9a584...ffd1a91. Read the comment docs.

@yeastplume
Copy link
Member Author

Update: This works end-to-end now, and (as advertised) produces transaction signatures without either party revealing their blinding factor (and with non-repudiation for either party).

Not complete yet, as I haven't looked at fixing up any tests that were run as a result, and there are a couple more features to add and cleanup to do. Particularly, I still need to ensure that there are multiple aggsig contexts allowed to support multiple transactions hitting the same wallet at once, and also decide when/how the receiver should decide a transaction isn't going to complete and remove the output from their wallet. But given this is still a first pass, these things can be tweaked/changed as we go along.

@antiochp @ignopeverell Probably in the state where you can start having a look/commenting on the changes (as and when you have time/inclination)

@yeastplume
Copy link
Member Author

Tests passing and ready to merge, if no objections. I'm going to open up separate issues for the tasks remaining, as I'd like to get all of the up to now changes merged.

This was referenced Jan 10, 2018
Copy link
Member

@antiochp antiochp left a comment

Choose a reason for hiding this comment

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

This looks great.
Liking how generic the various tx building steps are during wallet interaction.

Preparing myself mentally for the merge conflicts in #215 after we merge this 😬

👍

@yeastplume yeastplume merged commit 1199ed2 into mimblewimble:master Jan 10, 2018
@yeastplume yeastplume deleted the aggsig-transactions branch February 1, 2018 10:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants