feat(hermes-server): add option to allow reading the guardian set from ethereum#3399
feat(hermes-server): add option to allow reading the guardian set from ethereum#3399keyvankhademi wants to merge 8 commits intomainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
ali-behjati
left a comment
ali-behjati
left a comment
There was a problem hiding this comment.
I have two concerns here, first is to whether we need to keep Pythnet or not? (i think we can get rid of it).
Second is around the ETH contract we stick to here. the reason is that the WH main ETH is the trigger for the guardian transition and we are polling it every 60s and this means we can incur one minute of downtime when it happens. We might need to change it to another ETH contract [likely our custom one deployed in ETH or some other network].
p.s: In the future when we implement proper governance in Lazer, Lazer should become the source though.
| } | ||
|
|
||
| /// Makes an eth_call to a contract and returns the raw bytes | ||
| async fn eth_call( |
There was a problem hiding this comment.
i'm surprised that alloy doesn't have a function for this :?
There was a problem hiding this comment.
alloy has eth_call but I couldn't install full alloy because of a dependency issue with pinned solana sdk versions. Pavel is looking to see if there is a workaround.
| "Ethereum RPC address is required when guardian set source is 'ethereum'. \ | ||
| Set --wormhole-ethereum-rpc-addr or WORMHOLE_ETHEREUM_RPC_ADDR." | ||
| ) | ||
| })?; |
There was a problem hiding this comment.
errr i don't like doing validation like this here. isn't there any other way [like how does an enum work on clippy?]
Riateche
left a comment
Riateche
left a comment
There was a problem hiding this comment.
Added some minor suggestions.
| pub async fn fetch_guardian_sets_from_ethereum( | ||
| ethereum_rpc_url: &str, | ||
| wormhole_contract_addr: &str, | ||
| ) -> Result<(u32, GuardianSet, Option<(u32, GuardianSet)>)> { |
There was a problem hiding this comment.
It's unclear what's u32. Maybe replace the tuple with a struct with two fields?
| // Update current guardian set | ||
| Wormhole::update_guardian_set(&*state, current_index, current_set).await; | ||
|
|
||
| Wormhole::update_guardian_set(&*state, bridge.guardian_set_index - 1, previous).await; | ||
| // Update previous guardian set if available | ||
| if let Some((prev_index, prev_set)) = previous { | ||
| Wormhole::update_guardian_set(&*state, prev_index, prev_set).await; |
There was a problem hiding this comment.
I think it should be a single atomic update of both current and previous sets.
3 participants
Summary
This PR adds new config parameters to allow choosing
pythnetorethereumas a source for fetching the guardian set.Rationale
After migrating from pythnet, it will shutdown and will no longer be usable for fetching the guardian set.
How has this been tested?
Manually tested the fetched guardian set to be identical to pythnet's.