After #2100, the client only tracks the MMR peaks for the current sync height.
As mentioned here, I think there could be a problem with how we handle the ref_block on transaction execution.
Client::execute_transaction uses the current sync height as ref_block and passes it to the executor. The executor then builds the partial MMR through the data store. Since the client only stores peaks at the latest sync height, the MMR could not be consistent with the ref_block. If a concurrent sync_state advanced the chain between the caller capturing ref_block and the data store reading the peaks, the execution would fail.
I think there are two options here:
-
Assume a sync never runs during transaction execution. This would allow us to simplify the existing code by assuming the sync height is always the same during execution. It need to be documented (and ideally enforced).
-
Track historical peaks on the store. Allow blockchain_checkpoint to hold one row per chain tip we've seen, and look up peaks by ref_block. Since the client only accesses sync heigh, we only need to store the peaks for blocks that were added as chain tip, not for all intermediate blocks.
I think option 2 makes more sense and is probably the safest.
After #2100, the client only tracks the MMR peaks for the current sync height.
As mentioned here, I think there could be a problem with how we handle the
ref_blockon transaction execution.Client::execute_transactionuses the current sync height asref_blockand passes it to the executor. The executor then builds the partial MMR through the data store. Since the client only stores peaks at the latest sync height, the MMR could not be consistent with theref_block. If a concurrentsync_stateadvanced the chain between the caller capturingref_blockand the data store reading the peaks, the execution would fail.I think there are two options here:
Assume a sync never runs during transaction execution. This would allow us to simplify the existing code by assuming the sync height is always the same during execution. It need to be documented (and ideally enforced).
Track historical peaks on the store. Allow
blockchain_checkpointto hold one row per chain tip we've seen, and look up peaks byref_block. Since the client only accesses sync heigh, we only need to store the peaks for blocks that were added as chain tip, not for all intermediate blocks.I think option 2 makes more sense and is probably the safest.