Use tx_type in Coin and Snapshot instead of fIsCoinbase #856
Conversation
frolosofsky
force-pushed
the
tx-type-in-the-coin
branch
2 times, most recently
from
f23c526
to
8d4583f
Compare
frolosofsky
force-pushed
the
tx-type-in-the-coin
branch
from
0fc5be5
to
43f50d4
Compare
|
Rebased with master, fixed compilation errors when |
frolosofsky
force-pushed
the
tx-type-in-the-coin
branch
from
43f50d4
to
4abcf37
Compare
Signed-off-by: Stanislav Frolov <stanislav@thirdhash.com>
frolosofsky
force-pushed
the
tx-type-in-the-coin
branch
from
4abcf37
to
5e7351f
Compare
scravy
changed the title
Signed-off-by: Stanislav Frolov <stanislav@thirdhash.com>
Signed-off-by: Stanislav Frolov <stanislav@thirdhash.com>
|
@kostyantyn @Nizametdinov @Gnappuraz @scravy I've reacted on your comments and adjusted the code. Please take another look. |
Signed-off-by: Stanislav Frolov <stanislav@thirdhash.com>
There was a problem hiding this comment.
utACK 93d3ca7
There is a comment about the withdraw check which is not resolved #856 (comment) But since it's not related to this change and I noticed few other checks that are questionable, I think it's good to work on them separately and don't block this PR. I am currently reviewing esperanza protocol and will take care of such finalizer commit checks.
| @@ -175,23 +206,20 @@ bool ContextualCheckLogoutTx(const CTransaction &tx, CValidationState &err_state | |||
| // check (potentially goes to disk) and there is a good chance that if the | |||
| // vote is not valid (i.e. outdated) then the function will return before | |||
| // reaching this point. | |||
| CTransactionRef prev_tx; | |||
| uint256 block_hash; | |||
| TxType prev_tx_type = TxType::REGULAR; | |||
There was a problem hiding this comment.
This might result quite confusing. Since we are passing a pointer as an out parameter I would init as nullptr and then also change the FindPrevOutData signature to make it more evident (prev_tx_type_out). I would rather init this to TxType::REGULAR inside FindPrevOutData.
There was a problem hiding this comment.
bool FindPrevOutData(const COutPoint &prevout,
const CCoinsView &view,
TxType *tx_type_out,
CScript *script_out) {tx_type_out refers exactly to prevout outpoint. I think appending prev_ prefix would make it much more ambiguous.
Regarding nullptr initialization, pointers doesn't work in this way. I cannot initialize TxType with nullptr as it is not a pointer. I cannot pass nullptr to the FindPrevOutData because otherwise I cannot assign its value. We can pass pointer-to-pointer to the function (TxType **prev_tx_type) but then we will face with a problem of returning pointer to temporary (stack) variable — the Coin.
I suppose I could not understand your suggestion, feel free to elaborate a bit, then I will fix them in follow-up PR.
There was a problem hiding this comment.
I would have created a TxType* and initialized to nullptr and passed it to the function.
There was a problem hiding this comment.
Then it would not work unfortunately. We cannot write to the pointer, like *tx_type = TxType::REGULAR when tx_type is nullptr. We could use pointer-to-pointer (or reference-to-pointer), but I described why we can't.
Context
Currently, node cannot operate after fast-sync because it cannot validate finalizer commit inputs as these checks perform
GetTransactionthat leads to reading block from the disk; but node doesn't have such a block when running in fast-sync mode. Finally, node rejects finalizer commits and ban its peers. On the other hand, snapshot transfers UTXO set which has all needed transactions but there is a lack of transaction type in UTXO representation that prevents from using it.This PR fixes that by:
GetTransaction()inesperanza::ContextualCheck*;Fixes #547.