feat: solana implementation #13
Conversation
* Add Solana account parsing tools.
* Add solana verification functionality * Add unit tests to Solana message verifier * Refactor * Solana msg verifier reshape (#4) * remove comment * Use Solana types from Solana crates We were using hand crafted Solana types for interacting with RPC. * refactor: make clippy happy * Add some traces to Solana msg verifier * Remove outdated todos * refactor: re-use already calculated variable * Clarify test code usage * Solana msg verifier, try till one program log matches gw event * Add note to axelar to review when in PR * fail if tx_id is not found in signatures * Always fail on chain at handler level * Handle errors in case a tx cannot be found (finish the handler) * Remove commented code * Use original type from solana-axelar repo * Ensure that we verify sender address in sol msg verifier * Do not panic on PartialEq * refactor: clippy * remove no longer used code * Add more tests definition to solana msg verify iteration logic --------- Co-authored-by: eloylp <[email protected]>
Were only intended during development to understand the API.
Co-authored-by: tilacog <[email protected]>
Co-authored-by: tilacog <[email protected]>
Co-authored-by: tilacog <[email protected]>
Co-authored-by: tilacog <[email protected]>
Co-authored-by: tilacog <[email protected]>
Signed-off-by: Guilherme Felipe da Silva <[email protected]>
Signed-off-by: Guilherme Felipe da Silva <[email protected]>
It will be managed later. This prepares this branch for merge with upstream.
Cargo.toml
Outdated
There was a problem hiding this comment.
I see you've added a lot of dependencies across the Cargo.toml files. In my PR Axelar asked to remove as much dependencies as possible.
Just sharing.
There was a problem hiding this comment.
I have been trying to see how to minimize this. All dependencies are needed and are actually segregated in a consistent way, were the common ones belongs to the workspace and the specific ones, to just their crates.
I think the solution to the problem of dependencies needs to be though from an architectural point of view. In a way, we implement the ampd deps on our daemons, instead of the opposite. This was already subject of discussions in the past.
But maybe i am missing something. How did you solved the issue ? Maybe i can get inspiration :)
There was a problem hiding this comment.
I would double check if we really need all of these Solana ones:
solana-account-decoder = "=2.0.16"
solana-client = "=2.0.16"
solana-program = "=2.0.16"
solana-sdk = "=2.0.16"
solana-rpc-client-api = "=2.0.16"
solana-version = "=2.0.16"
solana-zk-token-sdk = "=2.0.16"
solana-transaction-status = "=2.0.16"
Because I have doubts that all of them are required
There was a problem hiding this comment.
True, i was assuming we had here some unused deps check. I will check it.
| @@ -15,7 +15,7 @@ pub trait Finalizer: Send + Sync { | |||
| async fn latest_finalized_block_height(&self) -> Result<U64>; | |||
| } | |||
|
|
|||
| #[derive(Debug, Deserialize, Serialize, PartialEq, Default)] | |||
| #[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Default)] | |||
There was a problem hiding this comment.
Do you need that change for the evm finalizer?
There was a problem hiding this comment.
Ah yeah, i think i can remember now. This is because the the Finalization struct is part of the Chain struct , which is part of the Config . Once we derive Clone in the parent (The Config) the res of the structs need to implement it as well. This was done because testing for being able to clone a config template in memory.
I honestly think its not bad to have that Clone there, as it favours testing. I would let them decide, but config is not an expensive thing to clone IMO.
There was a problem hiding this comment.
The derive(Clone) can be put behind a conditional test macro. But I believe that would only make the code uglier. Let's keep it unless we're told otherwise
There was a problem hiding this comment.
I don't know if it's intentional of you just missed, but this file has no unit tests.
There was a problem hiding this comment.
As far as i understood, the logic in mod.rs is common logic used by msg_verifier.rs and verifier_set_verifier.rs , which have their own unit tests. Initially i consider there's already good coverage there, but let me know @ctoyan @roberts-pumpurs if we want to add some more unit tests, even if they are a bit redundant.
There was a problem hiding this comment.
I agree that adding extra tests here is redundant and not worth the time
|
Okay, description updated, first feedback round addressed. Re requesting review ! |
bugfix on event parsing
At Eiger, we are implementing the integration between Axelar and Solana.
This PR introduces the necessary logic for the ampd component, including:
Both verifier components share the same Solana transaction fetch and log parsing logic, which facilitates identifying event sequences within Solana programs execution outputs.
Todos
Steps to Test
ampdwith an updatedampd.tomlfile, pointing to a localtofndsetup.tofndmust be up and running.ampddaemon.Expected Behaviour
It should forward GMP messages from one chain to the another, passing them thorugh the axelar devnet network.
Other Notes
Manual testing is missing because we are waiting for Axelar contract approval.