Make CBlock a vector of shared_ptr of CTransactions #1815
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
furszy
force-pushed
the
2020_block_to_shared_txs_pinter
branch
2 times, most recently
from
3cbe588
to
3009701
Compare
21 tasks
furszy
force-pushed
the
2020_block_to_shared_txs_pinter
branch
from
3009701
to
b1b7de0
Compare
random-zebra
added a commit
that referenced
this pull request
Sep 6, 2020
… clean up. 063401c PoS miner: add height to the first input scriptSig of the coinstake tx + remove unneded IncrementExtraNonce call in PoS blocks. (furszy) 7fc4680 Do not calculate the block value twice for the same block. (furszy) 7893818 Removing not needed CBlock::payee member. (furszy) 3e07280 Remove unused zPIVStake argument in FillBlockPayee. (furszy) f702628 miner: decouple coinbase transaction creation into its own function. (furszy) 377f87e FillBlockPayee: reworked to not require chainActive.Tip redundant access. (furszy) 920652b miner: remove FillBlockPayee unneded fee argument + refactor coinbase tx creation to be in one single place and not dispersed across the CreateNewBlock method. (furszy) Pull request description: Another step forward improving the miner `CreateNewBlock` function. This time, have unified the coinbase tx creation flow that was previously dispersed over the function and having some redundant initializations/sets. Plus, cleaned up some unused and/or unneeded fields and function arguments. This is a preparation for #1815 work. It will not be possible to modify any of the elements of a transaction once them are appended to a block, thus why the transaction needs (more than ever) to be crafted in one single place and not all over the sources. ACKs for top commit: Fuzzbawls: ACK 063401c random-zebra: utACK 063401c and merging... Tree-SHA512: c32cd87f82d9b31dedf9d47063a67978ef683bf35ff625823d6ece1a2d2904599dcbf15791682ffc1deb412082b1ecc24d11b364e300d4ceb259c65e65b0e9f6
furszy
force-pushed
the
2020_block_to_shared_txs_pinter
branch
from
b1b7de0
to
14243da
Compare
|
rebased |
|
Needs another rebase, due to a minor conflict with #1835 just merged. |
furszy
force-pushed
the
2020_block_to_shared_txs_pinter
branch
from
14243da
to
3b65314
Compare
furszy
changed the title
|
rebased again. |
random-zebra
added
Block Generation
random-zebra
previously approved these changes
Sep 19, 2020
furszy
force-pushed
the
2020_block_to_shared_txs_pinter
branch
from
3b65314
to
911e31c
Compare
|
done, rebased. |
Fuzzbawls
approved these changes
Sep 24, 2020
|
Note-to-self: This conflicts with #1869 |
random-zebra
added a commit
that referenced
this pull request
Sep 27, 2020
9d8b8a2 BugFix: CMutableTransaction following the same sapling nVersion rules as CTransaction. (furszy) 1ca1b89 PeerLogicValidation missing destructor declaration warning fix. (furszy) 093a766 Introduce convenience type CTransactionRef (furszy) 8345a87 Refactor: make the read function simpler (gnuser) Pull request description: PR solving the current master's syncing issue (syncing process not passing through block 5840), solved in 9d8b8a2. Essentially, when we merged #1815, we moved from using the default transaction constructor and the ser/unser template methods (`SerializationOp`) to be using, inside the templated serialization puzzle, the deserializing constructor (`CTransaction(deserialize_type, Stream& s)`) which internally creates a `CMutableTransaction` which wasn't having the same sapling tx version guard as `CTransaction` ser/unser method. So, in other words, it was trying to parse the shielded transaction data from an old version two transaction (yes, we already have version two transaction in our network.. first one is in block 5840). Plus, i took the mischief of not only including the bugfix, have added: * stream::read function readability improvement coming from bitcoin#11221. * a pretty straightforward adaptation of upstream's b4e4ba4 (missing last commit not included in #1815). * a `PeerLogicValidation` class compiler warning fix 1ca1b89 ACKs for top commit: random-zebra: ACK 9d8b8a2 Fuzzbawls: ACK 9d8b8a2 Tree-SHA512: ffb60001160d177d20da3d9198f73910921ec521e6cae1ccbf9f6359dc96e3e4fdbbcaabb85331ce2a8b839c07ab34b3981c69d83aa6990aa79e134588539f48
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Inspired on bitcoin#9125 (with many more adaptations), preparation for bitcoin#8580. Ant work 🐜
Establishing the bases for the work, we will need to continue migrating
CTransactiontostd::shared_ptr<const CTransaction>where is possible. Example bitcoin#8126 (can find more in #1726 list).TODO:
CTransactionRefimplementation commit.EDIT:
This is now depending on #1816 .