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: support multisend cosmos interface #143

Merged
merged 3 commits into from
Oct 17, 2024
Merged

feat: support multisend cosmos interface #143

merged 3 commits into from
Oct 17, 2024

Conversation

beer-1
Copy link
Member

@beer-1 beer-1 commented Oct 16, 2024

Description

Closes: #XXXX


Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

  • included the correct type prefix in the PR title, you can find examples of the prefixes below:
  • confirmed ! in the type prefix if API or client breaking change
  • targeted the correct branch
  • provided a link to the relevant issue or specification
  • reviewed "Files changed" and left comments if necessary
  • included the necessary unit and integration tests
  • updated the relevant documentation or specification, including comments for documenting Go code
  • confirmed all CI checks have passed

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

  • confirmed the correct type prefix in the PR title
  • confirmed all author checklist items have been addressed
  • reviewed state machine logic, API design and naming, documentation is accurate, tests and test coverage

Summary by CodeRabbit

  • New Features
    • Introduced a new function sudo_multisend for transferring fungible assets to multiple recipients simultaneously.
    • Allows users to specify recipient addresses and corresponding amounts in a single transaction.

@beer-1 beer-1 requested a review from a team as a code owner October 16, 2024 07:28
@beer-1 beer-1 self-assigned this Oct 16, 2024
Copy link

coderabbitai bot commented Oct 16, 2024

Walkthrough

The changes introduce a new public entry function called sudo_multisend in both the initia_std::coin and minitia_std::coin modules. This function enables the simultaneous transfer of fungible assets to multiple recipients, requiring parameters for the chain signer, sender signer, metadata, recipient addresses, and corresponding amounts. The implementation includes permission checks and utilizes vector operations for processing transfers. Additionally, the std::vector module is imported to support these operations.

Changes

File Path Change Summary
precompile/modules/initia_stdlib/sources/coin.move Added public entry function sudo_multisend for multiple asset transfers; imported std::vector.
precompile/modules/minitia_stdlib/sources/coin.move Added public entry function sudo_multisend for multiple asset transfers; imported std::vector.

Possibly related PRs

  • fix: metadata to denom #135: The changes in this PR modify the initia_std::coin module, which is directly related to the sudo_multisend function added in the main PR, as both involve the coin.move file and the handling of fungible assets.

Poem

In the meadow where bunnies play,
A new function hops in today! 🐇
With sudo_multisend, assets fly,
To friends and kin, oh my, oh my!
Hopping through vectors, swift and bright,
Sharing joy, what a lovely sight! 🌼


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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Outside diff range and nitpick comments (1)
precompile/modules/minitia_stdlib/sources/coin.move (1)

85-101: Approve with suggestions: sudo_multisend function implementation

The sudo_multisend function is a valuable addition for enabling batch transfers. However, I have a few suggestions to enhance its robustness and clarity:

  1. Add a check to ensure that the recipients and amounts vectors have the same length:
assert!(vector::length(&recipients) == vector::length(&amounts), error::invalid_argument(EINVALID_VECTOR_LENGTHS));
  1. Implement a balance check before processing transfers to ensure the sender has sufficient funds:
let total_amount = vector::fold(amounts, 0u64, |acc, amount| acc + amount);
assert!(balance(signer::address_of(sender), metadata) >= total_amount, error::invalid_argument(EINSUFFICIENT_BALANCE));
  1. Consider handling potential errors from individual transfers, perhaps by using a transaction script that can handle partial success.

  2. Add a comment explaining the purpose and administrative nature of the function:

/// Executes multiple transfers in a single transaction. This is an administrative
/// function that can only be called by authorized entities.
  1. Consider adding a maximum limit to the number of recipients to prevent potential DoS attacks or excessive gas usage.

These changes will improve the function's safety, clarity, and robustness.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between e492547 and 0350179.

📒 Files selected for processing (2)
  • precompile/modules/initia_stdlib/sources/coin.move (2 hunks)
  • precompile/modules/minitia_stdlib/sources/coin.move (2 hunks)
🧰 Additional context used
🔇 Additional comments (4)
precompile/modules/minitia_stdlib/sources/coin.move (2)

8-8: LGTM: Import of std::vector is correct and necessary.

The import of std::vector is appropriately placed and is required for the new sudo_multisend function.


Line range hint 1-101: Summary: New multisend functionality added with minor improvements suggested

The changes introduce a new sudo_multisend function to enable batch transfers of fungible assets. The implementation is generally well-structured and integrates smoothly with the existing codebase. However, I've suggested a few improvements to enhance safety, efficiency, and clarity.

Overall, this is a valuable addition to the module, providing a way to perform multiple transfers in a single transaction, which can be particularly useful for administrative or batch processing scenarios.

precompile/modules/initia_stdlib/sources/coin.move (2)

8-8: LGTM: New import added correctly.

The std::vector import is correctly added and is necessary for the new sudo_multisend function. The placement is consistent with the existing import style in the file.


Line range hint 1-478: Overall assessment: The changes implement multisend functionality as intended.

The new sudo_multisend function and the addition of the std::vector import successfully implement the multisend functionality for the Cosmos interface. The changes align well with the PR objectives and maintain consistency with the existing codebase.

Key points:

  1. The sudo_multisend function allows for efficient multiple transfers in a single transaction.
  2. Proper permission checks are in place using the check_sudo function.
  3. The implementation uses vector::zip_reverse for efficient iteration over recipients and amounts.

While the core functionality is sound, consider implementing the suggested improvements for input validation and error handling to enhance the robustness of the function.

precompile/modules/initia_stdlib/sources/coin.move Outdated Show resolved Hide resolved
@beer-1 beer-1 merged commit eee9b52 into main Oct 17, 2024
5 checks passed
@beer-1 beer-1 deleted the feat/multisend branch October 17, 2024 04:14
beer-1 added a commit that referenced this pull request Oct 21, 2024
* upgrade policy check to dependency modules

* fix broken tests

* pass policy check if dep is stdlib

* add code publish tests

* fmt and clippy

* feat: implement flashswap (#142)

* implement flashswap

* fix typo;

* fix missing pool_amounts conversion

* change the impl to use hot potato pattern

* fix typo and add borrow_amounts read function

* rename get_borrow_amounts

* movefmt

* lock the pool at flash swap and disallow provide liquidity and swap operations

* add testcases

* fmt and rebuild

* lock first before calling external functoins

* add flash swap event

* feat: support multisend cosmos interface (#143)

* introduce multisend

* remove unused from minitia_stdlib

* add comments for the error handlings

* change to call move function instead of reading table context data (#147)

* change to call move function instead of reading table context data

* lint

* change to do not receive module_ids

---------

Co-authored-by: beer-1 <[email protected]>
Co-authored-by: beer-1 <[email protected]>
@coderabbitai coderabbitai bot mentioned this pull request Oct 21, 2024
beer-1 added a commit that referenced this pull request Oct 30, 2024
* wip: apply loader v2, need to fix move to json error

* copy type conversion functions from runtime loader & handle json parsing error

* bump aptos core loader v2 version

* introduce InitiaModuleStorage using LRU module cache

* introduce InitiaCodeStorage to connect InitiaModuleStorage

* manage module cache as a single unit within the Initia VM

* manage script cache as a single unit within the Initia VM

* fmt and clippy

* compiled module & script interface change

* use custom move (#140)

* update rust version

* clone runtime_environment on every call

* introduce lock on cache (#141)

* use aptor-core move & flush type_cache every vm call

* not cloning environment with flushed ty_cache

* fix coderabbit audit

* feat: deps code upgrade policy check (#144)

* upgrade policy check to dependency modules

* fix broken tests

* pass policy check if dep is stdlib

* add code publish tests

* fmt and clippy

* feat: implement flashswap (#142)

* implement flashswap

* fix typo;

* fix missing pool_amounts conversion

* change the impl to use hot potato pattern

* fix typo and add borrow_amounts read function

* rename get_borrow_amounts

* movefmt

* lock the pool at flash swap and disallow provide liquidity and swap operations

* add testcases

* fmt and rebuild

* lock first before calling external functoins

* add flash swap event

* feat: support multisend cosmos interface (#143)

* introduce multisend

* remove unused from minitia_stdlib

* add comments for the error handlings

* change to call move function instead of reading table context data (#147)

* change to call move function instead of reading table context data

* lint

* change to do not receive module_ids

---------

Co-authored-by: beer-1 <[email protected]>
Co-authored-by: beer-1 <[email protected]>

* apply recent aptos update making struct index map to arc

* calculate cache size using real heap size

* fmt clippy

* add storage & code conversion test

* fix script access specifier

* add code scale test code to be used in e2e test & debug code to identify equal structs

* add panic handling when fetching size

* apply merged version

* Feat/refactor native code (#154)

* change upgrade policy u8 to enum

* code test cleanup & use loaderv2 as default

* code module type

* typo

* nit error message change

* cleanup module cache code

* nit error message

* delete type conversion funcs from sessionExt & use them from session directly

* use 1 as cache weight for all cache items

* minor lint fix

* feat: use constant cache weight for all modules & scripts (#156)

* use 1 as cache weight for all cache items

* small refactor

---------

Co-authored-by: beer-1 <[email protected]>
Co-authored-by: beer-1 <[email protected]>
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.

1 participant