Consensus: Refactor: Separate CheckFinalTx from main::IsFinalTx #6063
Conversation
There was a problem hiding this comment.
nit: To avoid this declaration, would it not be possible to move the implementation of CheckFinalTx() (L661) above isFinalTx() (above L648)? IIRC we don't have function declarations in main.cpp.
There was a problem hiding this comment.
This declaration will be necessary later anyway when the function is moved in jtimon@6769c02#diff-cbe22f30d7e480617350ef6ceca97d0cR65
So it's a temporary thing, and, yes, we have function declarations in main.cpp, for example, these two: https://github.com/bitcoin/bitcoin/blob/master/src/main.cpp#L81
The reason to make it in this way is to make the diff smaller.
|
Tested ACK. |
|
Properly rebased (I missed one recent change in IsFinalTx, the |
|
utACK You do not like BOOST_FOREACH? ;-) |
|
Updated without touching the foreach loop as discussed on IRC. |
|
utACK, this is contained in #6183 |
|
Well, not exactly, the names are changed in #6183 and the consensus-unfriendly one doesn't take any parameters besides the transaction. |
|
Closing after #6183 has been merged. |
|
@jtimon Feel free to fix that nit anyway - but we just had to move ahead to get the bug fix in. |
|
Yup, that's my thoughts here too. However give me a week or so and I'll do up that median time patch design so we can fix this exactly once. On "vacation" right now. :) On 2 June 2015 13:57:04 CEST, "Wladimir J. van der Laan" [email protected] wrote:
|
5 participants
A simple refactor as preparation for moving consensus to code for transaction validation.
The consensus version of IsFinalTx() cannot use chainActive.Height() or GetAdjustedTime()
This is part of #6051 but can be merged independently.