search

Meridian-IE / impact-evaluator

Impact Evaluator smart contract
Other
4 stars 0 forks source link

batch `setScores` updates #62

Closed juliangruber closed 1 year ago

juliangruber commented 1 year ago

Instead of updating the storage variables (expensive) for every participant, update once after processing all. Also cache a compution.

While making the whole contract slightly more expensive, this reduces setScores() cost for many participants by over 30%.

test_TransferScheduled() (gas: 3697 (0.146%)) 
test_SetScoresMultipleParticipants() (gas: 8979 (0.441%)) 
test_SetScoresFractions() (gas: 9601 (0.488%)) 
test_SetScoresMultipleCalls() (gas: 12041 (0.495%)) 
test_rewardsScheduledFor() (gas: 10223 (0.527%)) 
test_SetScores() (gas: 10223 (0.528%)) 
test_MinBalanceForTransfer() (gas: 10425 (0.538%)) 
test_ReleaseReward() (gas: 10223 (0.539%)) 
test_RewardBurner() (gas: 10653 (0.546%)) 
test_Reward() (gas: 10627 (0.547%)) 
test_AvailableBalance() (gas: 10653 (0.556%)) 
test_SetScoresOverflow() (gas: 10021 (0.561%)) 
test_SetScoresTooBig() (gas: 10021 (0.561%)) 
test_SetNextRoundLength() (gas: 10021 (0.561%)) 
test_AdvanceRound() (gas: 10021 (0.561%)) 
test_AdvanceRoundCleanUp() (gas: 10223 (0.563%)) 
test_SetScoresTooBigHistoric() (gas: 10223 (0.563%)) 
test_AddMeasurements() (gas: 10021 (0.567%)) 
test_SetScoresNotEvaluator() (gas: 10021 (0.569%)) 
test_SetRoundRewardNotAdmin() (gas: 10021 (0.569%)) 
test_SetScoresUnfinishedRound() (gas: 10021 (0.569%)) 
test_SetRoundReward() (gas: 10021 (0.570%)) 
test_SetScoresEmptyRound() (gas: 10845 (0.608%)) 
test_GasSetScores() (gas: 212021 (2.759%)) 
test_GasSetScoresManyParticipants() (gas: -62107583 (-23.635%)) 
Overall gas change: -61666737 (-19.620%)
juliangruber commented 1 year ago

Was able to remove one more state variable, bringing down gas overall

Screenshot 2023-11-13 at 16 04 15
juliangruber commented 1 year ago

Sorry I haven't addressed all of the review. Following up

juliangruber commented 1 year ago

While reviewing places accessing balanceHeld, I found that transferScheduled is updating that variable once for reach participant receiving the payment. Should we rework this part to be consistent with how we handle scoring?

I'm currently not concerned with that part, as it's not inside the setScores() path and therefore not (yet) burning a concerning amount of gas. Once we take a look at that, I agree this is a good thing to try

juliangruber commented 1 year ago

https://github.com/Meridian-IE/impact-evaluator/pull/65