Switch masternode id in Dash data structures from CTxIn to COutPoint#1933
Switch masternode id in Dash data structures from CTxIn to COutPoint#1933UdjinM6 merged 2 commits intodashpay:developfrom
Conversation
…(including p2p level)
UdjinM6
force-pushed
the
vintooutpoint
branch
from
35c0a39 to
9c56ebc
Compare
UdjinM6
changed the title
| | Field Size | Field Name | Data type | Description | | ||
| | ---------- | ----------- | --------- | ---------- | | ||
| | 41 | vin | [CTxIn](#ctxin) | The unspent output which is holding 1000 DASH | ||
| | 36 | outpoint | [COutPoint](#coutpoint) | The unspent output which is holding 1000 DASH |
There was a problem hiding this comment.
Nit: Field name is masternodeOutpoint in most other messages
There was a problem hiding this comment.
Yep, the corresponding class is a child of CMasternode which is a child of masternode_info_t and this field is defined there. Having masternodeOutpoint instead of simply outpoint might look a bit excessive (especially later in code when this field is used) that's why it was vin and not vinMasternode historically. But I'm open to changing it now and unifying the name across all classes/messages :) Let's see what @codablock thinks though.
There was a problem hiding this comment.
Ah, yep. I see what you mean. 👍
There was a problem hiding this comment.
Ok, I'll merge it as is now then. We can always fix it later if we change our minds :)
dash-docs/protocol-documentation.md
Outdated
| | Field Size | Field Name | Data type | Description | | ||
| | ---------- | ----------- | --------- | ---------- | | ||
| | 41 | vin | [CTxIn](#ctxin) | The unspent output which is holding 1000 DASH | ||
| | 36 | outpoint | [COutPoint](#coutpoint) | The unspent output which is holding 1000 DASH |
There was a problem hiding this comment.
Nit: Field name is masternodeOutpoint in most other messages
There was a problem hiding this comment.
Yep, this one is used only temporary in actual code and can be changed to masternodeOutpoint with no consequences to readability, I agree.
UdjinM6
force-pushed
the
vintooutpoint
branch
from
fb6c5dd to
abaef21
Compare
| { | ||
| CHashWriter ss(SER_GETHASH, PROTOCOL_VERSION); | ||
| ss << vinMasternode; | ||
| ss << masternodeOutpoint << uint8_t{} << 0xffffffff; |
There was a problem hiding this comment.
Any chance to get rid of this? Or at least add a comment about why it is there (I understand why it's there but only because of the PRs context).
There was a problem hiding this comment.
Can't easily get rid of it because this would break the hash and we have to keep votes for a long time. Could have an additional comment mentioning that we write these dummy values for legacy reasons, agree. Will add comments like that in another (cleanup) PR later.
…ashpay#1933) * Switch masternode id in Dash data structures from CTxIn to COutPoint (including p2p level) * outpoint -> masternodeOutpoint in DSEG
4 participants
This is not simply a refactoring, because it touches serialization (including the one on p2p level). Still, changes should be fully backwards compatible.