PR #2180 created some wrapper types to ensure that changes to the ChainStateView in the ChainWorkerState are rolled back on early termination. However, the heuristic I used to select which methods to move to the wrapper types was incomplete. I only checked the methods that had calls to chain.save() or chain.rollback(), but some methods still perform changes that were assumed to be dropped after the request completed.
Proposal
Use a broader and more cautious heuristic instead: every method that has a &mut self receiver and uses the ChainStateView's fields or methods must use one of the wrapper types. The only exception to the rule is the get_chain_state_view, which uses chain.clone_unchecked(). This is an exception because that method is derived on the ChainStateView and does not change it.
Test Plan
This bug was only found because the Matching Engine integration test failed in CI in PR #2163. After rebasing that changes from that PR on top of the changes from this PR, the test succeeds again.
Release Plan
Internal refactor and bug fix, nothing is needed except releasing a new patch version.
Motivation
PR #2180 created some wrapper types to ensure that changes to the
ChainStateViewin theChainWorkerStateare rolled back on early termination. However, the heuristic I used to select which methods to move to the wrapper types was incomplete. I only checked the methods that had calls tochain.save()orchain.rollback(), but some methods still perform changes that were assumed to be dropped after the request completed.Proposal
Use a broader and more cautious heuristic instead: every method that has a
&mut selfreceiver and uses theChainStateView's fields or methods must use one of the wrapper types. The only exception to the rule is theget_chain_state_view, which useschain.clone_unchecked(). This is an exception because that method is derived on theChainStateViewand does not change it.Test Plan
This bug was only found because the Matching Engine integration test failed in CI in PR #2163. After rebasing that changes from that PR on top of the changes from this PR, the test succeeds again.
Release Plan
Internal refactor and bug fix, nothing is needed except releasing a new patch version.
Links