I'd like to use the short id scheme for mempool syncing.
Refactor CBlockHeaderAndShortTxIDs::GetShortID into CTransaction #8277
pull pstratem wants to merge 1 commits into bitcoin:master from pstratem:2016-06-26-ctransaction-getshortid changing 2 files +11 −2-
pstratem commented at 5:26 AM on June 28, 2016: contributor
- pstratem force-pushed on Jun 28, 2016
-
Refactor CBlockHeaderAndShortTxIDs::GetShortID to CTransaction::GetShortID 740447d9fa
- pstratem force-pushed on Jun 28, 2016
- laanwj added the label Refactoring on Jun 28, 2016
-
petertodd commented at 7:16 PM on July 7, 2016: contributor
I'm a bit dubious about mixing in this non-consensus-critical code into the consensus-critical CTransaction, though I don't know what's the right alternative for you.
-
dcousens commented at 1:39 AM on July 8, 2016: contributor
weak NACK, but only from the point of view that I feel the context specific ID algorithms should remain, context specific. You've shown here how easy it would be to outline that new mempool syncing scheme just using
SipHashUint256directly. The two being the same is just consequential. -
laanwj commented at 6:23 AM on July 8, 2016: member
Tend to agree to keep this a separate function. In the long term we'd like the consensus critical data structures to be just data structures, with the bare minimum necessary for their interaction with consensus.
Network-code-specific functionality does not belong in there.
As you need it for mempool syncing you could move it to some other non-consensus utility header.
-
pstratem commented at 1:08 AM on July 11, 2016: contributor
Alight
- pstratem closed this on Jul 11, 2016
- DrahtBot locked this on Sep 8, 2021