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

Validate macrocoin, allow empty funds and IBC #190

Merged
merged 7 commits into from
Feb 8, 2024

Conversation

abefernan
Copy link
Contributor

@abefernan abefernan commented Jan 4, 2024

Closes #180.
Closes #183.
Closes #191.
Closes #192.

#180 was because of missing validation.
#183 and #192 because we didn't account for IBC tokens not disclosed on the chain-registry.
#191 because if funds/amount was an array, we sent an empty coin instead of an empty array.

@abefernan abefernan self-assigned this Jan 4, 2024
Copy link

vercel bot commented Jan 4, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
cosmos-multisig-ui ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 8, 2024 11:24am

@abefernan abefernan changed the title Add macrocoinToMicrocoin to validation Validate macrocoin, allow empty funds Jan 8, 2024
@abefernan abefernan changed the title Validate macrocoin, allow empty funds Validate macrocoin, allow empty funds and IBC Jan 8, 2024
});

console.log({ msgValue });
Copy link
Member

Choose a reason for hiding this comment

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

Should we remove this?

@@ -102,6 +107,13 @@ const MsgInstantiateContractForm = ({
return false;
}

try {
macroCoinToMicroCoin({ denom, amount }, chain.assets);
Copy link
Member

Choose a reason for hiding this comment

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

I'm confused by the name of this function because the case in #183 shows we have a base denom in atto.

Should this be renamed to displayCoinToBaseCoin or similar?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah the implementation has exceeded the logic behind the initial name I gave it

@abefernan abefernan merged commit 85cd225 into master Feb 8, 2024
5 checks passed
@abefernan abefernan deleted the fix/validate-microcoin branch February 8, 2024 11:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants