fix: change async_callback.id to number type
#170
Conversation
WalkthroughThe pull request introduces modifications to several modules, primarily focusing on the handling of asynchronous callbacks and the associated types. The Changes
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
precompile/modules/initia_stdlib/sources/minitswap.move (1)
3439-3439: Consider constructing JSON programmatically in testsHardcoding JSON strings in tests can be error-prone and less maintainable. Consider using JSON serialization functions to construct the expected JSON strings, enhancing readability and reducing potential errors.
Also applies to: 3458-3458
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
precompile/modules/initia_stdlib/sources/ibctesting/ibctesting.move(3 hunks)precompile/modules/initia_stdlib/sources/minitswap.move(4 hunks)precompile/modules/initia_stdlib/tests/ibc_transfer_tests.move(3 hunks)precompile/modules/minitia_stdlib/sources/ibctesting/ibctesting.move(3 hunks)precompile/modules/minitia_stdlib/tests/ibc_transfer_tests.move(3 hunks)
🔇 Additional comments (9)
precompile/modules/minitia_stdlib/sources/ibctesting/ibctesting.move (3)
208-208: LGTM!
The casting of async_callback.id from u32 to u64 ensures compatibility with the expected parameter type of dispatchable_ibc_timeout.
221-221: LGTM!
Casting async_callback.id from u32 to u64 ensures compatibility with dispatchable_ibc_ack.
404-406: LGTM!
Changing the id field from u64 to u32 addresses the JSON marshaling issue. Ensure that all parts of the code using MoveAsyncCallback are updated accordingly.
precompile/modules/initia_stdlib/sources/minitswap.move (2)
2401-2405: Confirm safe casting of batch_index to u32
In generate_ibc_message, batch_index of type u64 is cast to u32. Ensure that batch_index values remain within the u32 range to prevent data loss or overflow during the cast.
2340-2345:
Potential overflow risk after changing id from u64 to u32
Changing the id field in MemoAsyncCallbackV2 from u64 to u32 reduces the maximum allowable value from 18,446,744,073,709,551,615 to 4,294,967,295. Ensure that batch_index and any other sources of id do not exceed u32::MAX, otherwise this could lead to integer overflows or unexpected behavior.
Please verify that batch_index cannot exceed u32::MAX. If there is a possibility, consider implementing a mechanism to handle larger values or explore alternative solutions to retain u64 while addressing the JSON marshaling issue.
precompile/modules/initia_stdlib/sources/ibctesting/ibctesting.move (2)
404-404: Ensure consistency with id type change from u64 to u32
Changing the id field in MoveAsyncCallback from u64 to u32 requires verifying that all usages and related functions handle this change consistently. Confirm that other parts of the codebase that use id are compatible with this new type.
208-208: Validate safe casting from u32 to u64 in function calls
In relay_acks_timeouts, async_callback.id of type u32 is cast back to u64 when passed to dispatchable_ibc_timeout and dispatchable_ibc_ack. Ensure this casting does not introduce issues, and the functions correctly handle u32 values within a u64 parameter.
Also applies to: 221-221
precompile/modules/minitia_stdlib/tests/ibc_transfer_tests.move (1)
288-288: Update of id field in memo JSON is correct
The id field in the memo JSON has been updated from a string to a numeric value, reflecting the change to a u32 type in MoveAsyncCallback. The test cases have been appropriately adjusted.
Also applies to: 365-365, 444-444
precompile/modules/initia_stdlib/tests/ibc_transfer_tests.move (1)
288-288: Update of id field in memo JSON is correct
The changes correctly update the id field in the memo JSON from a string to a numeric value, aligning with the u32 type in MoveAsyncCallback. The test cases reflect this change accurately.
Also applies to: 365-365, 444-444
|
still do we need this? or is it okay to just close? |
After change json marshal, u64 type changed from number to string. This cause failure of unmarshal async callback msg.
async_callback.idtype to u32 on minitswap, ibc_transfer_test.Summary by CodeRabbit
New Features
IBCMemoV2,MemoMoveV2, andMemoAsyncCallbackV2to enhance compatibility with JSON marshaling.Improvements
idfield type in callback structures fromu64tou32for better type safety.relay_packetsfunction, factoring in both block height and timestamp.Bug Fixes
idformat in test cases from string to numeric for consistency in IBC transfer tests.Tests