feat: capo for ezETH on Arbitrum & Base #57
Conversation
| assertGe( | ||
| uint48(block.timestamp) - capAdapterParams.priceCapParams.snapshotTimestamp, | ||
| minimumSnapshotDelay, | ||
| "BlockUtils is underestimating the number of block per day for the network" | ||
| ); |
There was a problem hiding this comment.
not following why this was added here, can you elaborate?
There was a problem hiding this comment.
if blockutils is giving a too small block per day amount the setup of the retrospective test will fail with a very cryptic error as time isn't rewound back enouf.
There was a problem hiding this comment.
I think this can be removed as you have updated Arbitrum blocks per day to 345000
There was a problem hiding this comment.
to setup a retrospective test the contract will go back MINIMUM_SNAPSHOT_DELAY days back to create the contract with a ratio of 1 and query the real ratio. it will then go back to present and update the adapter with previously querried ratio. But this will fail if MINIMUM_SNAPSHOT_DELAY isn't separating the two event. This can fail as the second time is mesured using block.timestamp for time tracking while the first MINIMUM_SNAPSHOT_DELAY is a jump using block number, to convert from time to block a simple multiplication of the number of day in the variable and the amount of block per day on the chain (provided by blockutils) is used. Thus if blockutils is underestimating the number of block per day on said chain the test will revert later on with a cryptic error... this test catch this problem early on and give a clear(er) picture of what happened.
There was a problem hiding this comment.
I think it would be nyace to keep it, as there is talk to reduce block time on mainnet, and multiple l2s are still searching themselve so block count could update at anytime.
While it would be best to get rid of having to do this work manualy, I would argue that having this error show up next time an l2 update their block count make for a better develloper experience than whatever revert happen down the line.
Or maybe we can consider additional and optional test for blockutils 🤔
There was a problem hiding this comment.
yea it would make sense to have a separate one so we don't mix tests, we can leave it as it is for now
Co-authored-by: Ian Flexa <85500650+ianflexa@users.noreply.github.com>
Co-authored-by: Ian Flexa <85500650+ianflexa@users.noreply.github.com>
Co-authored-by: Ian Flexa <85500650+ianflexa@users.noreply.github.com>
No description provided.