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.

Sign up for GitHub

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

Jump to bottom

Always store and update kernel excess #231

Merged
merged 9 commits into from
Oct 3, 2019
Merged

Always store and update kernel excess #231

merged 9 commits into from
Oct 3, 2019

Conversation

yeastplume
Copy link
Member

@yeastplume yeastplume commented Oct 2, 2019 •
edited
Loading

Addresses #229

  • Always store and update the kernel excess when a transaction is created
  • Store kernel excess when receiving funds
  • Recalculate and store excess when confirming coinbase outputs
  • Only lookup by kernel if the transaction's amount_credited or amount_debited is zero
  • Use new Option serializer to output kernel excess, modify doctests
  • Output kernel excess on command line during grin-wallet txs call

Relies on mimblewimble/grin#3074

jaspervdm
jaspervdm reviewed Oct 2, 2019
View reviewed changes
Copy link
Contributor

@jaspervdm jaspervdm left a comment

Choose a reason for hiding this comment

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

Maybe we should also store the excess when receiving funds, in selection::build_recipient_output

@yeastplume yeastplume mentioned this pull request Oct 3, 2019
Merged
@yeastplume yeastplume added this to the 2.1.0 milestone Oct 3, 2019
@yeastplume
Copy link
Member Author

yeastplume commented Oct 3, 2019 •
edited
Loading

Good call, updated and top comment updated. Also had to update serializers, and recalculate the excess when confirming coinbase outputs, if you could just double check logic for that then I think we're good to go here.

jaspervdm
jaspervdm approved these changes Oct 3, 2019
View reviewed changes
Copy link
Contributor

@jaspervdm jaspervdm left a comment

Choose a reason for hiding this comment

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

Tested by sending some funds on floonet. Both the sender and receiver store the correct excess. From my side its ok to merge as soon as the azure checks pass.

@yeastplume yeastplume merged commit 2186900 into mimblewimble:master Oct 3, 2019
@yeastplume yeastplume deleted the store_excess branch October 21, 2019 12:45
yyangli pushed a commit to mwcproject/mwc-wallet that referenced this pull request May 13, 2020
@yeastplume
Always store and update kernel excess (mimblewimble#231)
2b7c2a2 
* always store and update kernel excess

* fix to parent key id on update as caught by testing

* save kernel excess for transaction recipients and coinbase outputs

* rustfmt

* logic cleanup, cargo.lock

* update to use node beta.3 release

* fix to doctests

* check result of calc offset before using

* rustfmt
antiochp pushed a commit to antiochp/grin-wallet that referenced this pull request Aug 7, 2020
@yeastplume
Always store and update kernel excess (mimblewimble#231)
c1ff232 
* always store and update kernel excess

* fix to parent key id on update as caught by testing

* save kernel excess for transaction recipients and coinbase outputs

* rustfmt

* logic cleanup, cargo.lock

* update to use node beta.3 release

* fix to doctests

* check result of calc offset before using

* rustfmt
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jaspervdm jaspervdm jaspervdm approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
2.1.0
Development

Successfully merging this pull request may close these issues.
2 participants
@yeastplume @jaspervdm