[V132-specific] RollupSwapSlip calls only once per block (in spawnDerivedAssets)
[V132-specific]
This is intended to close #1955 (closed)
'Uncontrolled growth of CurrentRollup'.
2024-04-24 update:
Coming back to this, I have simplified my original (total) diffs by call each RollupSwapSlip
from within spawnDerivedAssets
,
without duplicating getLayer1Assets
code calling.
This has the effect of reverting the changed TestSpawnDerivedAssets
unit test behaviour
(since multiple spawnDerivedAssets
calls would again imply multiple RollupSwapSlip
calls),
but at this time I have kept the earlier commits present in case of historical curiosity.
(Please tell me if commit squashing is preferred for reviewing.)
I also added a map[common.Asset]bool
to reassure myself that this would not be able to call RollupSwapSlip
for the same Asset if in future one were somehow an anchor for two different Assets.
[Update: The below is the initial description for if moving RollupSwapSlip
calls out of spawnDerivedAssets
to BeginBlock
.]
Following conversation with @pluto-9r , and related to !3496 (merged) and !3450 (merged),
this aims to fix the uncontrolled-currRollup-growth bug
(see the discussion threads of both other MRs)
by calling rollupSwapSlips
once in every NetworkMgr BeginBlock
(ideally rolling up once for each gas asset and once for each TOR Anchor asset, but feedback welcome)
and by in CalcAnchor
(called multiple times a block) calling GetCurrentRollup
(a read-online function) instead of RollupSwapSlip
(which adds the previous block's slip to the rollup).
Perhaps following merging of !3496 (merged) and observation that all is as intended (the discrepancy no longer incrementing),
store migration SetCurrentRollup
flushes will be appropriate (also resetting rollup counts to not then go negative,
or else (perhaps easier) DeletePoolSwapSlip
for some previous blocks to avoid becoming negative)
are appropriate to shed the accumulated excess rollup.
(Please note the TestSpawnDerivedAssets
unit test updates before and after.)