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

add lock_height to Transaction and TxKernel #167

Merged
merged 10 commits into from
Oct 11, 2017

Conversation

antiochp
Copy link
Member

@antiochp antiochp commented Oct 10, 2017

Initial attempt at introducing "time locked" transactions.


TODO -

  • rename transaction_fee_and_lock_height_to_bytes -> kernel_sig_msg
  • add tests to cover adding invalid tx to pool
  • add tests to cover building invalid block

  • add lock_height to Transaction and TxKernel
  • msg being signed in a kernel now includes both fee and lock_height
  • verify on kernel checks both fee and lock_height against the signature
  • wallet sender sets current chain height as the lock_height (as proof of concept)
  • pool checks the tx is valid (in terms of lock_height) before adding to pool
  • block is invalid if built with a tx with lock_height in future is included

Note: this is similar to the lock_height on a coinbase output but not the same.
This is a "timelock" on the tx/kernel itself, not an output.

See #25 for more info/context/discussion.

sign msg including both fee and lock_height in kernel
@antiochp
Copy link
Member Author

working on fixing up those pool test failures

/// consistently across Transaction and TxKernel
/// TODO - add some test coverage for this to make sure the bytes are in there
/// correctly
fn transaction_fee_and_lock_height_to_bytes(fee: u64, lock_height: u64) -> [u8; 32] {

This comment was marked as spam.

This comment was marked as spam.

@ignopeverell
Copy link
Contributor

Good in principle. I'm a little bummed we have to have another clear value in addition to fees but I don't have a better option for now.

@antiochp
Copy link
Member Author

@ignopeverell any further feedback/thoughts on this?
I added some test coverage for both adding timelocked txs to the pool and building a block from timelocked txs.

@ignopeverell
Copy link
Contributor

Looks good to me!

@ignopeverell ignopeverell merged commit bf7c1fb into mimblewimble:master Oct 11, 2017
@antiochp antiochp deleted the locktime_txns branch October 12, 2017 01:27
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.

2 participants