Minimize storage

[juliangruber]

This "radical" PR removes as much internal storage as possible, relying on Events as a method of cost saving and also reducing complexity. See gas comparison, before:

[PASS] test_AddMeasurements() (gas: 1911384)
[PASS] test_AdvanceRound() (gas: 1901709)
[PASS] test_SetNextRoundLength() (gas: 1899204)
[PASS] test_SetScores() (gas: 2007204)
[PASS] test_SetScoresEmptyRound() (gas: 1945811)
[PASS] test_SetScoresFractions() (gas: 2069759)
[PASS] test_SetScoresMultipleParticipants() (gas: 2125921)
[PASS] test_SetScoresNotEvaluator() (gas: 1855247)
[PASS] test_setRoundReward() (gas: 1850238)
[PASS] test_setRoundRewardNotAdmin() (gas: 1853482)

after:

[PASS] test_AddMeasurements() (gas: 1166367)
[PASS] test_AdvanceRound() (gas: 1230516)
[PASS] test_SetNextRoundLength() (gas: 1228349)
[PASS] test_SetScores() (gas: 1222263)
[PASS] test_SetScoresEmptyRound() (gas: 1190229)
[PASS] test_SetScoresFractions() (gas: 1269296)
[PASS] test_SetScoresMultipleParticipants() (gas: 1302958)
[PASS] test_SetScoresNotEvaluator() (gas: 1164214)
[PASS] test_setRoundReward() (gas: 1159622)
[PASS] test_setRoundRewardNotAdmin() (gas: 1162977)

In some calls, this is almost a 50% cost reduction. And since we only keep track of the current open rounds (ie rounds that have started but for which evaluation results haven't been submitted yet), which should on average just be 2, the contract's state won't grow.

Unless I missed something, the only impact this will have on our current running system is that we can't track per-round measurement count from the IE any more. This shouldn't be a problem, as we also have this state in the measurement and evaluate services. Listening for and querying events is already the main method of getting data out of the system. I also removed the method getParticipantScores, if a participant wants to see their score in a round they can look up the matching setScores() transaction.

[bajtos]

@juliangruber @patrickwoodhead I think this is such a major change that we should run it through another security audit, WDYT?

[juliangruber]

@juliangruber @patrickwoodhead I think this is such a major change that we should run it through another security audit, WDYT?

Agreed, once this and other changes have been merged, and running in stating for a bit, we should consider another audit

[bajtos]

@juliangruber could you please merge main and resolve the conflicts before we do the final review?

[AmeanAsad]

Hey @juliangruber trying to understand the openRounds concept here. Is there a scenario where we have two open rounds where evaluations are not submitted at the same time? Shouldn't the round process be synchronous?

The scenario im imagining is if the evaluations are being processed for one round and participants start submitting measurements for the following round?

[juliangruber]

Hey @juliangruber trying to understand the openRounds concept here. Is there a scenario where we have two open rounds where evaluations are not submitted at the same time? Shouldn't the round process be synchronous?

The scenario im imagining is if the evaluations are being processed for one round and participants start submitting measurements for the following round?

This can happen if the evaluation service errors/fails, but the new rounds are still created so that the system can continue functioning. Plus, you will always have at least two open rounds when you switch to the next one, since it takes a bit of time for evaluation results (which always come in after the round ends) to come in

[juliangruber]

I'm going to merge and deploy this now, so that we can start gathering usage data.

Hindsight review would be appreciated @bajtos @AmeanAsad 🙏