search

axelarnetwork / axelar-amplifier

Permissionless Connections Service run on the Axelar Network
21 stars 24 forks source link

feat: add support for Solana base58 msg id #494

Closed eloylp closed 3 months ago

eloylp commented 3 months ago

Description

At Eiger we are developing the Solana integration. During tests, we realised the current Base58TxDigestAndEventIndex does not support the specific Solana transaction ids (a.k.a as Signatures), which have a length of 88 characters on it's base58 encoded form.

This PR aims to add support for the Solana message id type's by adding a new enum variant on the existing codebase.

Todos

Steps to Test

Expected Behaviour

Other Notes

We are unsure about the non direct implications of this change nor it's impact into Axelar's future plans. We appreciate your feedback and suggestions.

eloylp commented 3 months ago

Seems the only fail in CI is likely only related to a rate limit:

[2024-07-09T10:30:05.182Z] ['error'] There was an error running the uploader: Error uploading to [https://codecov.io:](https://codecov.io/) Error: There was an error fetching the storage URL during POST: 429 - {'detail': ErrorDetail(string='Rate limit reached. Please upload with the Codecov repository upload token to resolve issue. Expected time to availability: 3276s.', code='throttled')}

Moving to "ready for review" !

eloylp commented 3 months ago

the changes look good. Could you update your branch and make the appropriate changes to the mod.rs in the msg_id module? We've made the change to hide it's submodules, so consumers only have to import axelar-wasm-std::msg_id to get access to all supported id types

Applied the changes, and also made our implementation to follow the rest of the code as indicated.

eloylp commented 3 months ago

There are also some refactor commits which improve naming, as it was a bit confusing before.