-
Notifications
You must be signed in to change notification settings - Fork 236
[INT-295] Feature Move Oapp example move vm decoding #1585
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
base: main
Are you sure you want to change the base?
Conversation
…coding features - Updated `evm-send.ts` to encode messages using `solidityPack` and log the encoded message. - Modified Aptos module to include a new `DecodedMessage` struct for storing received message details. - Added utility functions for byte and hex string conversions in a new `utils.move` file. - Implemented view functions to retrieve decoded message data from the Aptos module.
…ling - Renamed `DecodedMessage` struct to `ReceiveData` for clarity. - Simplified message parsing logic in `parse_message` function. - Updated `lz_receive_impl` to utilize the new `ReceiveData` struct. - Removed unused utility functions and constants from `utils.move`. - Added unit tests for `parse_message` to ensure correct functionality.
- Introduced a new script `analyze-decoded-values.ts` to analyze and log decoded values from the OApp on Aptos. - The script retrieves various decoded values and compares them against expected results, providing insights into potential decoding issues. - Removed the obsolete `aptos-move-get-count.ts` script to streamline the codebase.
…` script to streamline output. The script now focuses on essential decoded values without unnecessary commentary, enhancing clarity and conciseness.
…ieving decoded values. The script now accurately fetches and logs the raw message alongside other decoded values, enhancing the analysis output.
|
also |
- Updated `evm-send.ts` to use a specific contract address and renamed variables for better readability. - Enhanced logging to include network information and improved message formatting. - Moved constant definitions in `utils.move` to the top for better organization and removed duplicates to streamline the code.
- Revised README to reflect changes in Aptos CLI version requirements and added new testing instructions for OApp contracts. - Introduced new scripts `aptos-get-received-values.ts` and `evm-get-received-message.ts` for verifying cross-chain message delivery. - Enhanced `aptos-move-send.ts` to clarify OApp address configuration and improved logging for transaction fees. - Refactored `MyOApp.sol` to streamline message sending and receiving logic, ensuring better clarity and functionality.
…-send.ts` - Corrected the environment variable name from `ACCOUNT_ADDRESS` to `APTOS_ACCOUNT_ADDRESS` for consistency. - Updated the OApp address to a specific value for clarity in configuration. - Ensured all references to the account address are aligned with the new variable name.
…pp.test.ts` to streamline the test suite.
…-get-received-message.ts` - Added ABI encoding for messages in `aptos-move-send.ts` to ensure compatibility with Solidity contracts. - Improved logging for message sending and transaction links. - Updated placeholder addresses in both scripts for clarity and consistency. - Set a specific RPC URL in `evm-get-received-message.ts` for better connectivity.
- Changed example addresses and number to empty strings for user-defined input. - Improved clarity by indicating that users should fill in the required values for Aptos/Movement.
…ml` to streamline project dependencies.
| // OApp configuration | ||
| const OAPP_ADDRESS = '' // Set your OApp's address on Aptos | ||
| const REMOTE_EID = EndpointId.BSC_V2_TESTNET // Destination chain endpoint ID | ||
| const OAPP_ADDRESS = '<your-Aptos-Move-OApp-address>' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any reason not to use environment variable or config file for this? might be worth having a config.ts that just sets the oapp address, network, eids for the aptos and evm sends
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Its really just a personal preference thing. We don't usually offer this type of script in our devtools. I've just added it because I think it would be useful to have some scripts where people can play around and get an idea of how to call our contracts.
I think a config.ts file is overkill as this isn't supposed to be someone's workhorse. Just a simple hello world style script to get people started.
| /// Converts a hex string (without 0x prefix) to a vector of bytes | ||
| /// Example: "48656c6c6f" -> b"Hello" | ||
| /// Automatically pads odd-length strings with a leading zero | ||
| public fun hex_string_to_bytes(hex_str: String): vector<u8> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
generally, we wouldn't be sending a hex string over the wire. is this necessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in MyOApp.sol we do this:
// ──────────────────────────────────────────────────────────────────────────────
// 1. Send business logic
//
// Example: send a simple string to a remote chain. Replace this with your
// own state-update logic, then encode whatever data your application needs.
// ──────────────────────────────────────────────────────────────────────────────
/// @notice Send a string to a remote OApp on another chain
/// @param _dstEid Destination Endpoint ID (uint32)
/// @param _message The string to send
/// @param _options Execution options for gas on the destination (bytes)
function send(uint32 _dstEid, string calldata _message, bytes calldata _options) external payable {
// 1. (Optional) Update any local state here.
// e.g., record that a message was "sent":
// sentCount += 1;
// 2. Encode any data structures you wish to send into bytes
// You can use abi.encode, abi.encodePacked, or directly splice bytes
// if you know the format of your data structures
bytes memory _message = abi.encode(_message);
// 3. Call OAppSender._lzSend to package and dispatch the cross-chain message
// - _dstEid: remote chain's Endpoint ID
// - _message: ABI-encoded string
// - _options: combined execution options (enforced + caller-provided)
// - MessagingFee(msg.value, 0): pay all gas as native token; no ZRO
// - payable(msg.sender): refund excess gas to caller
//
// combineOptions (from OAppOptionsType3) merges enforced options set by the contract owner
// with any additional execution options provided by the caller
_lzSend(
_dstEid,
_message,
combineOptions(_dstEid, SEND, _options),
MessagingFee(msg.value, 0),
payable(msg.sender)
);
}
```
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm building this entire example around the existing OApp.sol so it will integrate into existing examples
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@St0rmBr3w what do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It may be worth just using a simpler data structure in the long-run, the string passing OApp is abi.encoded on EVM, which results in a complex data structure on non-EVM chains to unpack.
If you abi.encode("Hello World"), you will get the following byte structure:
// The function above will return the following raw bytes value.
0x0000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000000b48656c6c6f20576f726c64000000000000000000000000000000000000000000
// We can split this into words that are 32 bytes long to get:
0x0000000000000000000000000000000000000000000000000000000000000020 // offset
000000000000000000000000000000000000000000000000000000000000000b // length
48656c6c6f20576f726c64000000000000000000000000000000000000000000 // stringWhile "Hello World" makes sense for EVM chains, given the general complexity of implementing decoding for this message structure, I think it'd be better to have it just pass a uint value of some kind, and set a variable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nazreen got around this by changing the encoding format from the standard abi.encode, but I think this is generally more distracting from the point of the example: to show users how to simply encode a data structure and decode it on a non-EVM chain.
| bytes memory _message = abi.encodePacked(abi.encode(uint256(bytes(_string).length)), bytes(_string)); |
Co-authored-by: abaltes-lz <[email protected]>
…/devtools into move-oapp-example
…ssage types - Clarified the description of the unused _extraData parameter in the _lzReceive function. - Updated the comment for the SEND constant to specify its role in sending arbitrary strings and the potential for different message types with enforced options.
- Simplified the validation of required environment variables by using constants for `APTOS_PRIVATE_KEY` and `APTOS_ACCOUNT_ADDRESS`. - Improved code readability and maintainability by reducing redundancy in the validation logic.
…rity - Removed the unused `bytes_to_string` function from `utils.move` to streamline the module. - Updated `hex_string_to_bytes` to enforce even-length hex strings, improving error handling. - Simplified the logic in `hex_string_to_bytes` and `bytes_to_hex_string` for better readability.
…app.move and utils.move - Introduced `parse_message` function in `oapp.move` to extract addresses and a u256 number from a cross-chain message. - Enhanced documentation for the new function to clarify the message format. - Organized error codes in `utils.move` for better readability and maintainability.
…arity and efficiency - Replaced manual byte slicing and conversion logic with utility functions `extract_address` and `extract_u256` for better readability and maintainability. - Simplified the extraction process by utilizing a mutable position tracker.
…ncy and readability - Replaced manual byte extraction with a mutable position tracker and utility function for extracting u256 values. - Streamlined the logic for slicing the message vector, improving overall clarity and maintainability.
…for code clarity - Removed unused imports from `oapp.move` and `utils.move` to streamline the modules. - Cleaned up the test file `oapp_tests.move` by eliminating unnecessary dependencies, enhancing readability.
…and clarity - Updated `MyOApp.sol` to replace the `lastMessage` string with `address1`, `address2`, and `num` for better data representation. - Modified scripts to align with the new data structure, ensuring proper encoding and retrieval of the updated fields. - Enhanced comments in utility scripts to clarify the purpose of the changes and improve overall readability.
|
|
||
| contract MyOApp is OApp, OAppOptionsType3 { | ||
| constructor(address _endpoint, address _delegate) OApp(_endpoint, _delegate) Ownable(_delegate) {} | ||
| /// @notice Last string received from any remote chain |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this comment seems inaccurate
it mentions "last string received"
while below there are three variables: address, address, uint256 and none of them is string
maybe better to change comment to "last data received"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is taken directly from the EVM OApp example (that devrel wrote) I'm just updating these files with their implementation. If you want me to change it across all examples I can do that, but it should be a separate PR.
| _dstEid, | ||
| _message, | ||
| combineOptions(_dstEid, SEND, _options), | ||
| MessagingFee(msg.value, 0), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is an anti pattern to force users to use native gas for paying fee and hardcoding it in the code.
This makes it impossible in future to pay with ZRO token, I don't think we want to encourage users to follow this pattern and disable it in their OApps they are building on top of this example.
Instead, send method should accept MessagingFee struct, like OFTCore: https://github.com/LayerZero-Labs/devtools/blob/main/packages/oft-evm/contracts/OFTCore.sol#L177
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is taken directly from the EVM OApp example (that devrel wrote) I'm just updating these files with their implementation. If you want me to change it across all examples I can do that, but it should be a separate PR.
| // 1. Decode the incoming bytes into a string | ||
| // You can use abi.decode, abi.decodePacked, or directly splice bytes | ||
| // if you know the format of your data structures | ||
| (address1, address2, num) = abi.decode(_message, (address, address, uint256)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does it work if you are currently encoding a "string" on send:
bytes memory _message = abi.encode(_message);
and on receive you are decoding "address, address, uint"?:
(address1, address2, num) = abi.decode(_message, (address, address, uint256));
I did a Foundry small test and I think if you currently do abi.encode offchain, pass it onchain and then do abi.encode again in Solidity before sending it results in wrong data:
function test_msg_passing() public virtual {
bytes memory encoded = abi.encode(string(abi.encode(address(1), address(2), 100)));
console.logBytes(encoded);
(address address1, address address2, uint256 num) = abi.decode(encoded, (address, address, uint256));
console.log(address1);
console.log(address2);
console.log(num);
}
result:
Logs:
0x00000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000060000000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000064
0x0000000000000000000000000000000000000020
0x0000000000000000000000000000000000000060
1
Expected:
0x000000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000064
0x0000000000000000000000000000000000000001
0x0000000000000000000000000000000000000002
100
Could you create a test case in Foundry for sending messages from EVM to EVM using this OApp Solidity code and double check if the encoding and decoding logic is consistent and works?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good. Putting this on pause until sla-tracker work is finished.
Beefing up the move oapp example with useful parameter encoding sent from evm-send.ts
Adding some utils on move side to assist people in parsing data. I think its useful to have amore built out move example here because most partners come form evm land and have no idea what they're doing in move vm land. The current implementation was built for a partner.