Simplify IsFinalTx logic. #19518

There is no need to explicitly check tx.nLockTime == 0. If tx.nLockTime is 0, then it will be compared with nBlockHeight, which will only fail if nBlockHeight is 0. nBlockHeight is only 0 for the Genesis block (see https://github.com/bitcoin/bitcoin/blob/f4de89edfa8be4501534fec0c662c650a4ce7ef2/src/validation.cpp#L3544) The Genesis block is fixed and its sequence number is final. Therefore the IsFinalTx will still return true, even if it is called on the Genesis block.

<!-- *** Please remove the following help text before submitting: *** Pull requests without a rationale and clear improvement may be closed immediately. GUI-related pull requests should be opened against https://github.com/bitcoin-core/gui first. See CONTRIBUTING.md -->

<!-- Please provide clear motivation for your patch and explain how it improves Bitcoin Core user experience or Bitcoin Core developer experience significantly: * Any test improvements or new tests that improve coverage are always welcome. * All other changes should have accompanying unit tests (see `src/test/`) or functional tests (see `test/`). Contributors should note which tests cover modified code. If no tests exist for a region of modified code, new tests should accompany the change. * Bug fixes are most welcome when they come with steps to reproduce or an explanation of the potential issue as well as reasoning for the way the bug was fixed. * Features are welcome, but might be rejected due to design or scope issues. If a feature is based on a lot of dependencies, contributors should first consider building the system outside of Bitcoin Core, if possible. * Refactoring changes are only accepted if they are required for a feature or bug fix or otherwise improve developer experience significantly. For example, most "code style" refactoring changes require a thorough explanation why they are useful, what downsides they have and why they *significantly* improve developer experience or avoid serious programming bugs. Note that code style is often a subjective matter. Unless they are explicitly mentioned to be preferred in the [developer notes](/doc/developer-notes.md), stylistic code changes are usually rejected. -->

<!-- Bitcoin Core has a thorough review process and even the most trivial change needs to pass a lot of eyes and requires non-zero or even substantial time effort to review. There is a huge lack of active reviewers on the project, so patches often sit for a long time. -->

This change appears to implement identical logic except in the cases noted.

My preference would be if we're going to be cleaning up this code, to clean up the gross ternary expression to be easier to read rather than a (modified/validated genesis block excluded) mostly redundant check.

Is there a motivating reason for this change other than code cleanup?

My personal motivation is that I was trying to interpret the locktime rules in order to work with them. I replicated the logic as it was written here originally, but upon reflection it seemed unnecessarily complex. I cleaned up the logic in my own work and thought I might as well suggest the same clean up for Bitcoin Core while I was on the topic.

Might be interesting to check out a previous PR I opened which cleaned up consensus logic for similar reasons. #15969 (comment), as it discusses a large number of contributors perceived priorities around simplifying consensus logic.

I'm a Concept NACK on changes like this.

While this patch seems correct to me, I think that changing consensus code should have a pretty high bar -- if there is no discernible benefit to the change (such as performance, or allowing for development of new features), then I'd prefer that we (at most) document our understanding of the logic in cases like this, rather than change it.

If our documentation ends up being wrong, no harm done; if instead an incorrect change to consensus code slips through, we risk splitting the network. As a project, I don't think it makes sense to spend the effort that would be required to make the latter a sufficiently low-risk choice when compared with the former. Our efforts are better spent on adding features and fixing bugs.

Too controversial for a simple fix like this. Closing for now.