-
Notifications
You must be signed in to change notification settings - Fork 2
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: Allows locking multiple denoms #61
Conversation
amount: lock_entry.funds.amount, | ||
}; | ||
|
||
response = response.add_message(BankMsg::Send { |
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 guess the performance would be better if we had only one BankMsg::Send with multiple tokens?
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.
Not sure if it makes any difference, since this is processed by the wasm vm anyways, right?
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 is actually handled by the x/wasm module that needs to dispatch each individual message to the proper cosmos module, which then handles it. So current code means that send() will be handled N times by the x/bank module. But we can keep it like this for now.
Left some comments, but otherwise LGTM. We will make some changes to |
Perfect, thanks. Yeah, this PR has some placeholder implementations right now that I anticipate we'll eventually work together to make real :) |
This PR includes these changes: