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

multi: Implement enforced relative sequence locks. #864

Merged
merged 4 commits into from
Sep 22, 2017

Conversation

davecgh
Copy link
Member

@davecgh davecgh commented Sep 20, 2017

This PR contains several commits with the overall purpose of implementing consensus-enforced relative lock-time semantics and a new opcode OP_CHECKSEQUENCEVERIFY for the script system that provides the ability to conditionally enforce relative time-based restrictions.

First, it repurposes the sequence number of version 2 transaction inputs to provide consensus-enforced relative lock-time semantics so that a transaction can require inputs to have a specified relative age before they are allowed to be included in a block. Each relative time lock can either specify a relative number of seconds (with a granularity of 512 seconds and a maximum value of 33,553,920) or a specific number of blocks (max 65535).

The number of seconds is calculated relative to the past median time of the block before the one that contains the referenced output. This is done because, due to other changes that will also be included in the same agenda vote, said time will become the earliest possible time the block that contains the referenced output could have been (technically it will be one second after that, but that complexity is ignored since there is already a granularity involved anyways).

It is also possible to disable the behavior by setting bit 31 of the sequence number (which all transactions currently do by default since they are set to the max sequence number).

In order for the transaction to be permitted to the mempool, relayed, considered for inclusion into block templates, and allowed into a block, the specified relative time locks for all of its inputs must be satisfied.

In order to accomplish this new behavior, the concept of a sequence lock is introduced which allows the minimum possible time and height at which a transaction can be included into a block to be calculated from all inputs with non-disabled relative time locks, and functions to calculate and evaluate the sequence lock are added.

Second, it modifies the script engine to replace OP_NOP3 with OP_CHECKSEQUENCEVERIFY and adds a flag to selectively enable its enforcement named ScriptVerifyCheckSequenceVerify.

The new opcode examines the top item on the stack and compares it against the sequence number of the associated transaction input in order to allow scripts to conditionally enforce the inclusion of relative time
locks to the transaction.

As of this PR, the new sequence lock behavior and OP_CHECKSEQUENCEVERIFY opcode are only enforced when considering candidate transactions for acceptance to the mempool, relaying, and inclusion into block templates. The maximum standard transaction version is also raised to version 2 accordingly. This is acceptable because they are soft forking changes and the aforementioned areas only enforce policy.

A consensus vote is required in order to reject blocks which contain transactions that violate the new rules at a consensus level. Code to selectively enable consensus enforcement based on the result of an agenda vote will be added in a separate commit.

The following is an overview of the changes:

  • Introduce a new struct named SequenceLock to represent the previously described sequence lock
  • Define new constants related to sequence numbers named SequenceLockTimeDisabled, SequenceLockTimeIsSeconds, SequenceLockTimeMask, and SequenceLockTimeGranularity
  • Add a new function named calcSequenceLock to calculate the sequence lock for a given transaction
  • Add a new function named SequenceLockActive to determine if a given sequence lock is satisfied for a given block height and past median time
  • Add a convenience function named LockTimeToSequence which can be used to convert a relative lock time to a sequence number
  • Add a comprehensive set of tests for all of the new sequence lock funcs
  • Introduce a constant named OP_CHECKSEQUENCEVERIFY which has the same value as OP_NOP3 since it is replacing it
    • Update opcode to name mappings accordingly
  • Introduce a new flag named ScriptVerifyCheckSequenceVerify to provide conditional enforcement of the new opcode
  • Abstract the logic that deals with time lock verification since it is the same for both the new opcode and OP_CHECKLOCKTIMEVERIFY
  • Implement the required opcode semantics
  • Add tests to ensure the opcode works as expected including when used both correctly and incorrectly
  • Modify mempool to consider version 2 transactions standard
  • Introduce a new callback to the mempool config for obtaining the sequence lock for block after the current best chain tip so it can remain decoupled from the chain
  • Update mempool to enforce relative locks on all candidate transactions
  • Update the mock chain in the mempool test harness accordingly

@davecgh davecgh force-pushed the multi_relative_sequence_locks branch 2 times, most recently from b01a2fa to 393b366 Compare September 20, 2017 09:35
@davecgh davecgh added this to the v1.1.0 milestone Sep 20, 2017
Copy link
Member

@dajohi dajohi left a comment

Choose a reason for hiding this comment

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

OK

@davecgh davecgh force-pushed the multi_relative_sequence_locks branch 3 times, most recently from e02651b to d7fd04f Compare September 21, 2017 20:11
@marcopeereboom
Copy link
Member

I think I may want to marry this code!

}

// Mask off the value portion of the sequence number to obtain
// the time lock delta required before this this input can be
Copy link
Member Author

Choose a reason for hiding this comment

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

s/this this/this/

medianTime: now + 30,
want: false,
},

Copy link
Member Author

Choose a reason for hiding this comment

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

Spurious blank line.

This repurposes the sequence number of version 2 transaction inputs to
provide consensus-enforced relative lock-time semantics so that a
transaction can require inputs to have a specified relative age before
they are allowed to be included in a block.  Each relative time lock can
either specify a relative number of seconds (with a granularity of 512
seconds and a maximum value of 33,553,920) or a specific number of
blocks (max 65535).

The number of seconds is calculated relative to the past median time of
the block before the one that contains the referenced output.  This is
done because, due to other changes that will also be included in the
same agenda vote, said time will become the earliest possible time the
block that contains the referenced output could have been (technically
it will be one second after that, but that complexity is ignored since
there is already a granularity involved anyways).

It is also possible to disable the behavior by setting bit 31 of the
sequence number (which all transactions currently do by default since
they are set to the max).

In order for the transaction to be permitted to the mempool, relayed,
considered for inclusion into block templates, and allowed into a block,
the specified relative time locks for all of its inputs must be
satisfied.

This only implements the required logic and tests to enforce the new
behavior.  Code to enforce the new behavior when considering candidate
transactions for acceptance to the mempool, relaying, and inclusion into
block templates will be added in a separate commit.

A consensus vote is required in order to reject blocks which contain
transactions that violate the new rules at a consensus level.  Code to
selectively enable consensus enforcement based on the result of an
agenda vote will be added in a separate commit.

In order to accomplish this new behavior, the concept of a sequence lock
is introduced which allows the minimum possible time and height at which
a transaction can be included into a block to be calculated from all
inputs with non-disabled relative time locks, and functions to calculate
and evaluate the sequence lock are added.

The following is an overview of the changes:

- Introduce a new struct named SequenceLock to represent the previously
  described sequence lock
- Define new constants related to sequence numbers named
  SequenceLockTimeDisabled, SequenceLockTimeIsSeconds,
  SequenceLockTimeMask, and SequenceLockTimeGranularity
- Add a new function named calcSequenceLock to calculate the sequence
  lock for a given transaction
- Add a new function named SequenceLockActive to determine if a given
  sequence lock is satisfied for a given block height and past median
  time
- Add a convenience function named LockTimeToSequence which can be used
  to convert a relative lock time to a sequence number
- Add a comprehensive set of tests for all of the new funcs
This adds enforcement of the newly introduced relative time locks via
transaction sequence numbers when considering candidate transactions for
acceptance to the mempool, relaying, and inclusion into block templates.
It also raises the maximum standard transaction version to 2
accordingly.  This is acceptable because it is a soft forking change and
the aforementioned areas only enforce policy.

The following is an overview of the changes:

- Modify mempool to consider version 2 transactions standard
- Introduce a new callback to the mempool config for obtaining the
  sequence lock for block after the current best chain tip so it can
  remain decoupled from the chain
- Update mempool to enforce relative locks on all candidate transactions
- Update the mock chain in the mempool test harness accordingly
This modifies the script engine to replace OP_NOP3 with
OP_CHECKSEQUENCEVERIFY and adds a flag to selectively enable its
enforcement.

The new opcode examines the top item on the stack and compares it
against the sequence number of the associated transaction input in order
to allow scripts to conditionally enforce the inclusion of relative time
locks to the transaction.

The following is an overview of the changes:

- Introduce a new flag named ScriptVerifyCheckSequenceVerify to
  provide conditional enforcement of the new opcode
- Introduce a constant named OP_CHECKSEQUENCEVERIFY which has the same
  value as OP_NOP3 since it is replacing it
  - Update opcode to name mappings accordingly
- Abstract the logic that deals with time lock verification since it is
  the same for both the new opcode and OP_CHECKLOCKTIMEVERIFY
- Implement the required opcode semantics
- Add tests to ensure the opcode works as expected including when used
  both correctly and incorrectly
This updates the default policy for standard transactions to enforce the
new CSV opcode.  In other words, with this change, the new opcode will
be enforced when considering candidate transactions for acceptance to
the mempool, relaying, and inclusion into block templates.
@davecgh davecgh force-pushed the multi_relative_sequence_locks branch from d7fd04f to ed9f64b Compare September 21, 2017 20:59
// See the CalcSequenceLock comments for more details.
//
// This function MUST be called with the chain state lock held (for writes).
func (b *BlockChain) calcSequenceLock(node *blockNode, tx *dcrutil.Tx, view *UtxoViewpoint, isActive bool) (*SequenceLock, error) {
Copy link
Member

Choose a reason for hiding this comment

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

should this always be the best node? seems to be called only with it, and would make the code clearer if that was explicitly spelled out either in the function comment or the variable name.

Copy link
Member

Choose a reason for hiding this comment

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

question arises because it uses that node's height + 1 to calculate the mempool height

Copy link
Member Author

@davecgh davecgh Sep 22, 2017

Choose a reason for hiding this comment

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

It has to be an arbitrary node because it is called for reorganizations by the consensus code based on the result of the required consensus vote (PR #855).

It is indeed only ever called from the mempool with the best node though, so the +1 will only apply in that case.

@davecgh davecgh merged commit ed9f64b into decred:master Sep 22, 2017
@davecgh davecgh deleted the multi_relative_sequence_locks branch September 22, 2017 07:05
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