Skip to content

Conversation

gmaxwell
Copy link
Contributor

@gmaxwell gmaxwell commented Nov 1, 2015

As observed by Luke-Jr, BIP113's behavior would permit some transactions which are currently invalid. E.g. when the current block time is higher than the MedianTimePast and the transaction's locktime is between the block time and the MTP.

There appears to be a straight-forward fix of using MAX(blocktime, MTP+offset) but this deserves some consideration before implementation.

… endpoint for lock-time calculations"

This reverts commit 9d55050.

As noted by Luke-Jr, under some conditions this will accept transactions which are invalid by the network
 rules.  This happens when the current block time is head of the median time past and a transaction's
 locktime is in the middle.

This could be addressed by changing the rule to MAX(this_block_time, MTP+offset) but this solution and
 the particular offset used deserve some consideration.
@sipa
Copy link
Member

sipa commented Nov 2, 2015

ACK

@laanwj
Copy link
Member

laanwj commented Nov 2, 2015

Ugh.
utACK

@laanwj laanwj added the Mempool label Nov 2, 2015
@fanquake
Copy link
Member

fanquake commented Nov 2, 2015

utACK

@laanwj laanwj merged commit 40cd32e into bitcoin:master Nov 2, 2015
laanwj added a commit that referenced this pull request Nov 2, 2015
40cd32e Revert "Add rules--presently disabled--for using GetMedianTimePast as endpoint for lock-time calculations" (Gregory Maxwell)
8537ecd Revert "Enable policy enforcing GetMedianTimePast as the end point of lock-time constraints" (Gregory Maxwell)
@maaku
Copy link
Contributor

maaku commented Nov 2, 2015

So since the logic to revert in the first place was incorrect, any chance we can revert the reversion?

@laanwj
Copy link
Member

laanwj commented Nov 2, 2015

We should, but I'm still recovering from panic here.

@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants