Skip to content

BIP68/113 for generation transaction#415

Merged
luke-jr merged 1 commit intobitcoin:masterfrom
jl2012:bip68_113_coinbase
Jul 6, 2016
Merged

BIP68/113 for generation transaction#415
luke-jr merged 1 commit intobitcoin:masterfrom
jl2012:bip68_113_coinbase

Conversation

@jl2012
Copy link
Copy Markdown
Contributor

@jl2012 jl2012 commented Jul 5, 2016

To clarify the applicability of BIP68 and 113 to generation transaction

@luke-jr
Copy link
Copy Markdown
Member

luke-jr commented Jul 5, 2016

@maaku
Copy link
Copy Markdown
Contributor

maaku commented Jul 5, 2016

This isn't true. The rules apply as much to outputs of a coinbase as they do any other transaction. Is the proposed text talking about the coinbase's nSequence value? Then that should be specified exactly, although I don't think the clarification is necessary (coinbase is already specially treated).

@jl2012
Copy link
Copy Markdown
Contributor Author

jl2012 commented Jul 5, 2016

@maaku, isn't the SequenceLocks is done only for non-coinbase tx?

https://github.com/bitcoin/bitcoin/blob/master/src/main.cpp#L2480

And we do need to clarify as some miners are using the nSequence as extranonce
(I'm talking about the BIP68, not BIP112)

@luke-jr
Copy link
Copy Markdown
Member

luke-jr commented Jul 5, 2016

nSequence is about inputs, not outputs... and the new rule for it doesn't make sense for the generation tx input since it's not spending coins.

@afk11
Copy link
Copy Markdown
Contributor

afk11 commented Jul 5, 2016

For 113 (MTP), the generation transaction's locktime is checked, so looks good to me: https://github.com/bitcoin/bitcoin/blob/master/src/main.cpp#L3602

@jl2012 Maybe clarify by saying 68 doesn't apply to the inputs of the coinbase transaction? It can certainly apply to spends of it's outputs.

@jl2012 jl2012 force-pushed the bip68_113_coinbase branch from ce6d962 to 24eaeed Compare July 5, 2016 16:39
@jl2012
Copy link
Copy Markdown
Contributor Author

jl2012 commented Jul 5, 2016

@maaku @afk11 @luke-jr Edited


When the relative lock-time is block-based, it is interpreted as a minimum block-height constraint over the input's age. A relative block-based lock-time of zero indicates an input which can be included in any block. More generally, a relative block lock-time n can be included n blocks after the mining date of the output it is spending, or any block thereafter.

The new rules are not applied to the nSequence field of the input of generation transaction.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

"of the generation"

Is this the proper terminology? Should we say "coinbase transaction" here? Honestly not sure what the current accepted terminology is.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

"coinbase tx" is definitely more common.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The coinbase is the input scriptSig for the generation transaction.

@maaku
Copy link
Copy Markdown
Contributor

maaku commented Jul 5, 2016

Right, the sequence lock checks are not done for the coinbase input. The updated text is better (see comment made)

@jl2012 jl2012 force-pushed the bip68_113_coinbase branch from 24eaeed to 418d497 Compare July 5, 2016 17:52
@jl2012 jl2012 force-pushed the bip68_113_coinbase branch from 418d497 to 8d40b6e Compare July 5, 2016 17:53
@jl2012
Copy link
Copy Markdown
Contributor Author

jl2012 commented Jul 5, 2016

@maaku @btcdrak fixed

@afk11
Copy link
Copy Markdown
Contributor

afk11 commented Jul 5, 2016

ACK 8d40b6e

2 similar comments
@btcdrak
Copy link
Copy Markdown
Contributor

btcdrak commented Jul 5, 2016

ACK 8d40b6e

@maaku
Copy link
Copy Markdown
Contributor

maaku commented Jul 5, 2016

ACK 8d40b6e

@luke-jr
Copy link
Copy Markdown
Member

luke-jr commented Jul 6, 2016

Disagree with "coinbase transaction", but this is hardly the first time, and an ACK is an ACK...

@luke-jr luke-jr merged commit ad2af69 into bitcoin:master Jul 6, 2016
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.

5 participants