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.
The problem was located in
Block::accumulate_transaction
, which is invoked repeatedly by a miner as he selects which mempool transactions to include in his block. Recall that the block has only one transaction, which represents the merger of all transactions of the miner's choice.The problem is that the block-in-preparation contains the updated mutator set accumulator. When a new transaction is aggregated, the mutator set accumulator is updated from this intermediate state, with the new transaction's addition and removal records. It is not obvious why this leads to a problem, so let's use an example:
inputs_1
andoutputs_1
and the current block-in-preparation has mutator set accumulatormsa_1
, which is what results from applyingoutputs_1
andinputs_1
to the previous block's mutator set accumulator:msa_1 = msa_0.add(outputs_1).remove(inputs_1)
.inputs_2
andoutputs_2
. Theaccumulate_transaction
(up until now) computes the new mutator set accumulator asmsa_1.add(outputs_2).remove(inputs_2)
.msa_1
.So how do we get them in sync? We can't use the archival mutator set because we might not be running an archival node. We can't look at the new mutator set accumulator because we might need the chunk of the SWBF that was slid away and is now hidden by Merkleization. We can't revert the removal of
inputs_1
because we still need that slid chunk to restore the mutator set accumulator.The solution in this PR is to modify the type signature of the function
accumulate_transaction
, which now takes an additional argumentprevious_mutator_set_accumulator
. The new mutator set is computed by starting from this value and applying all addition records (outputs_1 || outputs_2
) first and all removal records (inputs_1 || inputs_2
) second. The halfway-in-between mutator set accumulator is ignored.At some point we should consider a prettier solution at some point -- one that does not generate objects that are destined to be ignored. However, that sounds to me like an optimization with low priority and low payoff. It affects only miners (who need way more memory anyway) and it's not clear whether it can run any faster if it is implemented in this way.
This PR also:
H
from all structs and functions associated with the mutator set. Throughout this code base we only ever useTip5
which itself is already hidden behind type aliasHash
. The genericity dates to the time where we needed an alternative to the relatively slow Rescue-Prime hash functino.Result<(),_>
from functions where there is no control path leading to error. Accordingly catch-and-report-error clauses where these functions are called, are removed as well.