Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: encode source address correctly in gateway #1548

Merged
merged 7 commits into from
Sep 14, 2023

Conversation

mustermeiszer
Copy link
Collaborator

Description

The address that we are receiving from EVM chains is a utf-8(hex(address)) encoded byte bloob. This was an issue before because we expected it to be just an [u8; 20].

Changes and Descriptions

  • update parsing of account from Axelar
  • utility function for both precompile and gateway

Checklist:

  • I have added Rust doc comments to structs, enums, traits and functions
  • I have made corresponding changes to the documentation
  • I have performed a self-review of my code
  • I have added tests that prove my fix is effective or that my feature works

cdamian
cdamian previously approved these changes Sep 13, 2023
@mustermeiszer mustermeiszer added the D0-ready Pull request can be merged without special precaution and notification. label Sep 14, 2023
cdamian
cdamian previously approved these changes Sep 14, 2023
Copy link
Contributor

@wischli wischli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM apart from two minor details which can also be ignored.

@@ -12,6 +12,7 @@ targets = ["x86_64-unknown-linux-gnu"]

[dependencies]
codec = { package = "parity-scale-codec", version = "3.0.0", features = ["derive"], default-features = false }
hex = {version = "0.4.3", default-features = false }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CR: We are importing hex without std here but not enabling the std feature below.

Comment on lines 390 to 395
// NOTE: Axelar simply provides the hexadecimal string of an EVM
// address as the `sourceAddress` argument. Solidity does on the
// other side recognize the hex-encoding and encode the hex bytes to
// utf-8 bytes.
//
// Hence, we are reverting this process here.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Formatting off.

Suggested change
// NOTE: Axelar simply provides the hexadecimal string of an EVM
// address as the `sourceAddress` argument. Solidity does on the
// other side recognize the hex-encoding and encode the hex bytes to
// utf-8 bytes.
//
// Hence, we are reverting this process here.
// NOTE: Axelar simply provides the hexadecimal string of an EVM
// address as the `sourceAddress` argument. Solidity does on the
// other side recognize the hex-encoding and encode the hex bytes to
// utf-8 bytes.
//
// Hence, we are reverting this process here.

@mustermeiszer mustermeiszer enabled auto-merge (squash) September 14, 2023 09:57
Copy link
Contributor

@wischli wischli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for applying my nits!

@mustermeiszer mustermeiszer merged commit 5d09fe7 into main Sep 14, 2023
10 checks passed
@NunoAlexandre NunoAlexandre deleted the feat/decode-axelar-source-address branch September 14, 2023 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
D0-ready Pull request can be merged without special precaution and notification.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants