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: Inbound Transaction Verification #785

Merged
merged 5 commits into from
Feb 28, 2024
Merged

Conversation

benjamin852
Copy link
Contributor

  1. Added docs for evm + cosmos gmp tx verification
  2. Added warning for sending txs sent to axelar chain (which we do not yet support)

@milapsheth if you have time feel free to review the cosmos gmp verification to ensure it's accurate. Thanks

Copy link

vercel bot commented Feb 20, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
axelar-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 28, 2024 1:27pm

@benjamin852 benjamin852 changed the title Inbound Transaction Verification feat: Inbound Transaction Verification Feb 20, 2024
@StephenFluin
Copy link
Collaborator

@benjamin852 can you be clear with the state of this PR? I saw code pushed, but is it finished, are all comments addressed? Any desired eng reviews complete?

@benjamin852
Copy link
Contributor Author

@benjamin852 can you be clear with the state of this PR? I saw code pushed, but is it finished, are all comments addressed? Any desired eng reviews complete?

I don't see any comments here that need to be addressed?
PR is ready to be merged.
Would be nice to get someone from eng to review the cosmos section here but don't think the whole pr needs to be blocked until then, we could just iterate on it after if someone wants me to change something there.

src/layouts/navigation.ts Show resolved Hide resolved
@@ -13,6 +13,11 @@ With GMP, you can:
**NOTE:** The security of your contracts is limited to the security of the chains they integrate with. Since blockchains can have different security practices, we recommend doing due diligence on all chains your contract will be deployed to.
</Callout>

<Callout emoji="🚨">
**NOTE:** GMP Transactions using `callContractWithToken` sent to the Axelar chain itself are not yet supported. Axelar only supports GMP call without token from Axelar to chain X. Transactions sent to the Axelar chain as the destination chain will be stuck until support is rolled out.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are we planning on supporting this? I thought this wasn't part of our architecture?

Chain x I think you need to be more specific, like "supported chains" or something? We don't support any GMP from or to Axelar. (Axelar is not a supported GMP chain, right?).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a feature the team wants to add

src/pages/dev/general-message-passing/verify-gmp-tx.mdx Outdated Show resolved Hide resolved
src/pages/dev/general-message-passing/verify-gmp-tx.mdx Outdated Show resolved Hide resolved
3. `SourceAddress`: The source address of the contract call
4. `PayloadHash`: A hash of the GMP message that was sent with the call

Using this information it verifies that the inbound call has indeed been verified by the Axelar network and that this data is the authentic data passed in from the source chain rather than a malicious execution with invalid data on the destination chain.
Copy link
Collaborator

Choose a reason for hiding this comment

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

"verifies it has indeed been verified" language is a little unclear cc @ffe9f8

@benjamin852 benjamin852 merged commit b46aecd into main Feb 28, 2024
2 checks passed
@benjamin852 benjamin852 deleted the ben/tx-verification branch February 28, 2024 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants