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

Enforce nLockTime in the blockchain #1820

Closed
gmaxwell opened this issue Sep 12, 2012 · 3 comments
Closed

Enforce nLockTime in the blockchain #1820

gmaxwell opened this issue Sep 12, 2012 · 3 comments
Labels

Comments

@gmaxwell
Copy link
Contributor

    bool IsFinal(int nBlockHeight=0, int64 nBlockTime=0) const
    {
        // Time based nLockTime implemented in 0.1.6
        if (nLockTime == 0)
            return true;   
        if (nBlockHeight == 0)
            nBlockHeight = nBestHeight;
        if (nBlockTime == 0)
            nBlockTime = GetAdjustedTime();
        if ((int64)nLockTime < ((int64)nLockTime < LOCKTIME_THRESHOLD ? (int64)nBlockHeight : nBlockTime))
            return true;
        BOOST_FOREACH(const CTxIn& txin, vin)
            if (!txin.IsFinal())
                return false;   
        return true;
    }

IsFinal() can never return false. The final return true should be return false and the input check should come before the time check (or left where it is and had its sense reversed). Would be a trivial fix except its a soft-forking change. ... and will probably require bumping the transaction version number as well as updating the block enforcing rules.

It's well known that replacement didn't work, but nLockTime not being enforced in the chain was a surprise to me

@gmaxwell
Copy link
Contributor Author

13e100dd08b6da0a7426ea520b0bb3ae54cef79dd045e2e4f7116023df3a5c95 is an example of a violating transaction in the chain.

@sipa
Copy link
Member

sipa commented Sep 12, 2012

How do you mean IsFinal() can never return false? If you want a non-final transaction, at least one transaction input should have a non-maximum nSequence number.

@gmaxwell
Copy link
Contributor Author

Doh. Missed that it was calling the ctxin IsFinal not itself.

@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

No branches or pull requests

2 participants