You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Found by adversarial review of #9, filed so it does not ambush the first log-tables move.
Not a #9 blocker: the failure mode is a loud CI wedge, not silent wrongness, and it cannot
fire until this repo has BOTH a frozen release and a subsequent table move.
The hazard
test/src/abstract/DecimalFloatDeploySnapshot.t.sol and test/src/abstract/DecimalFloatDeployChain.t.sol etch only the current candidate log
tables in setUp():
RainDeployVerifySnapshot / RainDeployVerifyChain derive every suite in releasedSuites() as well as the candidates.
Deriving a released DecimalFloat runs its frozen creation code, whose constructor
checks the log tables at the address it was compiled against.
setUp() plants tables only at the current candidate address. Today those coincide, so
everything passes.
The first rain-math-float bump that moves the log tables breaks the coincidence: the new
candidate address gets etched, the old released DecimalFloat's constructor still checks
the old address, nothing plants anything there, and every derivation of that released suite
reverts DeployFailed.
Consequence
testSnapshotInternallyConsistent and the chain test go permanently red for a release
that is genuinely live on chain — against an append-only record whose whole premise is
that released suites stay checkable forever. The machinery is fine; the test scaffolding
cannot honor it on this repo's one known-fragile axis.
The fix
Extend both setUp()s to plant tables for every released log-tables@* suite, not
only the candidate: iterate LibLogTablesReleased.releasedSuites() and Zoltu-deploy (or
etch at the derived address) each frozen tables record, then the candidate. A released
DecimalFloat then always finds the tables its constructor expects.
Sequencing: must land before the second release if the first table move comes with it —
in practice, any time before rain-math-float next moves the tables.
Two comment fixes to fold in (LOW/INFO from the same review)
script/lib/LibEtchLogTables.sol:10-13 still says it is "Used by script/Deploy.sol"; Migrate deploy records to src/generated/ #9 removed that use (the new Deploy refuses rather than etches). Comments describe
current behaviour only.
script/Build.sol:126-133 says regeneration "converges on the second run". On a real
table move it is run three: run 1 unchanged (compiled-in old pointers), run 2 writes the
new LogTables candidate then crashes DeployFailed (DecimalFloat's compiled-in check
still names the old address), run 3 converges. Loud either way; the comment should say
what actually happens.
Found by adversarial review of #9, filed so it does not ambush the first log-tables move.
Not a #9 blocker: the failure mode is a loud CI wedge, not silent wrongness, and it cannot
fire until this repo has BOTH a frozen release and a subsequent table move.
The hazard
test/src/abstract/DecimalFloatDeploySnapshot.t.solandtest/src/abstract/DecimalFloatDeployChain.t.soletch only the current candidate logtables in
setUp():RainDeployVerifySnapshot/RainDeployVerifyChainderive every suite inreleasedSuites()as well as the candidates.DecimalFloatruns its frozen creation code, whose constructorchecks the log tables at the address it was compiled against.
setUp()plants tables only at the current candidate address. Today those coincide, soeverything passes.
The first
rain-math-floatbump that moves the log tables breaks the coincidence: the newcandidate address gets etched, the old released
DecimalFloat's constructor still checksthe old address, nothing plants anything there, and every derivation of that released suite
reverts
DeployFailed.Consequence
testSnapshotInternallyConsistentand the chain test go permanently red for a releasethat is genuinely live on chain — against an append-only record whose whole premise is
that released suites stay checkable forever. The machinery is fine; the test scaffolding
cannot honor it on this repo's one known-fragile axis.
The fix
Extend both
setUp()s to plant tables for every releasedlog-tables@*suite, notonly the candidate: iterate
LibLogTablesReleased.releasedSuites()and Zoltu-deploy (oretch at the derived address) each frozen tables record, then the candidate. A released
DecimalFloat then always finds the tables its constructor expects.
Sequencing: must land before the second release if the first table move comes with it —
in practice, any time before
rain-math-floatnext moves the tables.Two comment fixes to fold in (LOW/INFO from the same review)
script/lib/LibEtchLogTables.sol:10-13still says it is "Used byscript/Deploy.sol";Migrate deploy records to src/generated/ #9 removed that use (the new Deploy refuses rather than etches). Comments describe
current behaviour only.
script/Build.sol:126-133says regeneration "converges on the second run". On a realtable move it is run three: run 1 unchanged (compiled-in old pointers), run 2 writes the
new LogTables candidate then crashes
DeployFailed(DecimalFloat's compiled-in checkstill names the old address), run 3 converges. Loud either way; the comment should say
what actually happens.