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

Calls from Solana have incorrect sender address in message context #3689

Open
fadeev opened this issue Mar 11, 2025 · 3 comments · May be fixed by #3700
Open

Calls from Solana have incorrect sender address in message context #3689

fadeev opened this issue Mar 11, 2025 · 3 comments · May be fixed by #3700
Assignees

Comments

@fadeev
Copy link
Member

fadeev commented Mar 11, 2025

Describe the Bug

When making a call (or a deposit) from Solana, message context's origin value does not contain the actual Solana address from which the call was made. Instead, the sender value (which so far is being used only for calls from EVM chains) contains a hex address.

To Reproduce

Make a call from Solana to a contract that reverts, for example:

https://zetachain-testnet.blockscout.com/address/0xCec4d6c0528557dbF45dC8cF31189A8D1c32f428

This contract always reverts, so that we can see message context in the error_message of a CCTX.

npx hardhat solana-deposit-and-call \
  --amount 0.001 \
  --recipient 0xCec4d6c0528557dbF45dC8cF31189A8D1c32f428 \
  --types '["string"]' alice    

https://zetachain-athens.blockpi.network/lcd/v1/public/zeta-chain/crosschain/inboundHashToCctxData/38yC7sTor9kgdsXg8fMWGbBmRMbjTF9ZTCrCuoipxiKNBu7Z6TRYiiLuP9397KDGZzSuXLvuKK5ztC9oLM4rtEfx

{[] 0x73543467664e774b5a396A666F786A4773346164 901}

Expected Behavior

Solana sender address should be as the first arg of message context (origin) and the second value (sender) should be empty.

@fadeev fadeev added the bug Something isn't working label Mar 11, 2025
@lumtis lumtis self-assigned this Mar 12, 2025
@lumtis
Copy link
Member

lumtis commented Mar 12, 2025

We use sender field as EVM address for all chain with v2 deposits, Bitcoin as well, origin field is never set. We had a conversation some time ago I think about the usage of the field.

I would see doing the following change:

  • Keep the first bytes attribute in MessageContext to keep backward compatibility in any case but rename it to NonEVMSender , if we use it for non EVM chain origin name doesn't make sense because it would be filled by the contract doing the deposit, not the origin signer of the transaction, I don't think there is a single use case of using the origin signer? In general tx.origin usage is strongly not advised to be used
  • NonEVMSender bytes is filled if the source chain is non evm
  • Sender address is filled when the source chain is evm so dev directly have access to an address value
  • Dev must implement their dispatching logic but we could offer library that retrieve a "chain" enum from the chain ID in the message context

Alternative

  • deprecate the second sender address attribute
  • Rename the first as sender bytes and use it for all chain, even EVM, the dev must convert it to address if needed.
    Might be seen cleaner but more work to convert the address + would break compatibility with contract using the sender currently, I would prefer solution 1

@lumtis
Copy link
Member

lumtis commented Mar 12, 2025

Adding the issue to #3651 to be addressed this sprint

@lumtis lumtis removed the bug Something isn't working label Mar 12, 2025
@lumtis
Copy link
Member

lumtis commented Mar 12, 2025

Solution discussed:

  • Rename origin into sender
  • Set sender for all chains including EVM
  • Rename old sender into senderEVM
  • Still set senderEVM to same value

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 a pull request may close this issue.

2 participants