I01 validate state history upon initialization #2092
Conversation
alysiahuggins
requested review from
nomaxg,
philippecamacho,
ImJeremyHe,
sveitser,
jbearer,
tbro and
imabdulbasit
as code owners
contracts/src/LightClient.sol
Outdated
| // Whereas blockCommRoot can be zero, if we use special value zero to denote empty tree. | ||
| // feeLedgerComm can be zero, if we optionally support fee ledger yet. | ||
| // The viewNum and blockHeight in the genesis state must be zero to indicate that this is | ||
| // the |
There was a problem hiding this comment.
This comment block is strangely formatted.
| if ( | ||
| _genesis.viewNum != 0 || _genesis.blockHeight != 0 | ||
| || BN254.ScalarField.unwrap(_genesisStakeTableState.blsKeyComm) == 0 | ||
| || BN254.ScalarField.unwrap(_genesisStakeTableState.schnorrKeyComm) == 0 | ||
| || BN254.ScalarField.unwrap(_genesisStakeTableState.amountComm) == 0 | ||
| || _genesisStakeTableState.threshold == 0 | ||
| || _genesisStakeTableState.threshold == 0 || _stateHistoryRetentionPeriod < 1 hours |
There was a problem hiding this comment.
Can we define a constant for the minimum period of 1 hours and then use that here and in setstateHistoryRetentionPeriod?
There was a problem hiding this comment.
sure, we can define a constant. Historically, I've been trying to minimize the use of constants in upgradeable contracts to avoid storage layout issues (even though it's stored in the contract's bytecode, if the same variable is no longer a constant in a future version of the contract, it would mess up the storage layout). However, since we use the inheritance pattern for handling upgrades, we shouldn't run into that issue. Will proceed
There was a problem hiding this comment.
we spoke offline and agreed not to set a constant since constants can't be upgraded and this constant is one that does have the possibility of changing. Instead, extra comments were added to ensure that developers and users are aware of the min state history retention period 58f43df
… hour and update script to perform permissionedProver validation correctly
alysiahuggins
changed the title
Closes #2072
This PR:
This PR does not:
Key places to review:
initializeStatemethodHow to test this PR:
forge test --match-test test_revertWhen_InvalidStateHistoryRetentionPeriodOnSetUpThings tested