Hi all! Thanks for responses and feedback - really appreciate it!
@Hasu +1 - totally agree on the importance of solid processes, and appreciate the flexibility!
there’s no time pressure to implement things fast, actually - they won’t be incorporated fast, there’s time to spare for polish
That’s good to know, thanks!
we can afford a code that would use a more precise way to understand what validators are a part of Lido set than checking by withdrawal credentials
the hack used to compute the balance (relying on withdrawal credentials) is not fully reliable; e.g. it would fail to deliver correct value on the testnet now, because making a deposit with Lido withdrawal credentials is permissionless
Thanks for pointing out! In general, withdrawalCredentials check was used as “the most reliable out of simple solutions” - and unfortunately as you pointed out it’s not reliable enough. There is a range of more complex ones that can achieve higher reliability - ranging from additional oracles, to stateful contracts (+something to update their state), to changes in Lido protocol - each with different tradeoffs and risks. Happy to provide more insights into the challenges, potential options and their tradeoffs/risks, let me know if that would be helpful and what’s the best format for it (e.g. comment here, separate thread, shared doc, video call, etc.)
It probably would not work on the testnet and, more importantly, it can lead to a DoS attack if someone were to set up the validator with Lido credentials but without involving the Lido contract. The Lido contract checks for the invariant
deposited_validators >= reported_validatorsupon report, and if it fails, no reports would go through.
Acknowledged. Yes, it doesn’t tally on testnet - there are already some old validators on testnet that use Lido withdrawalCredentials, but not part of the Lido protocol (at least from the perspective of keys-api).
active_validatorsandexited_validatorsare not the correct values to be reported by the oracle. The current report provides the number of all visible Lido validators on the Beacon chain side, including active , exited and even validators not activated yet.
Good point - it is a relatively simple change to make. Re: “not activated yet” validators - the actual check is “activation_eligibility_epoch <= current_epoch” - if I’m not wrong this means “validator have deposited eth and is in the activation queue, or activated”. Anyway, for this logic I assumed lido-oracle accounting module being a “reference implementation” and what I ended up with tallied with accounting module report (on multiple runs on multiple days). This might mean that I haven’t hit a edge case though, so appreciate your insight on how it should behave.
Using the term
TVLfor the sum of Lido validators’ balances might be confusing
Sure, would CL prefix be a good replacement (i.e. clBalance, ZKCLOracleContract, etc.)?
Overall, I’m excited to see this proposal and eager to assist to make it live from the side of the Protocol team.
Thanks!
But I considered the approach from the further perspective of complete replacement of our conventional oracle as the proposal outlines. And if we want it, we need some other method to prove inclusion.
Definitely - there are quite a few things before this could completely replace the conventional oracle. I think the approach of using zk-oracle as an additional sanity check to the conventional oracle (with some “relaxed” checks, as @vsh noted) allows capturing some value and learnings earlier, and gradually enhance precision, add other reported fields, etc.
I’d rather make it simple for an MVP and concentrate on
clBalancemainly
I think if making a check between zk and conventional oracle incurs some unwanted complexity/overhead/etc., this could be left for future enhancement. On the other hand, It is valuable (at least from “obtain learnings” perspective) to have it reported and proved by zk-oracle, so I think the best course of action is to keep it reported by zk-oracle.