-
Notifications
You must be signed in to change notification settings - Fork 13
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: Implement token wrapper library and complete token tester #7
Conversation
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.
How is it tested or how do you plan to test it?
docker run --rm -v "$(pwd)":/code \ | ||
--mount type=volume,source="devcontract_token_tester",target=/code/contracts/staking/target \ | ||
--mount type=volume,source=registry_cache,target=/usr/local/cargo/registry \ | ||
cosmwasm/rust-optimizer:0.9.0 ./contracts/token-tester |
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.
why not 0.9.1?
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.
The other contract of v0.10.0 uses 0.9.0, so it is combined.
I'm going to add a test using linkwasmd/cli which is included in link-modules. |
) -> StdResult<Binary> { | ||
let res = LinkTokenQuerier::new(&deps.querier) | ||
.query_balance(contract_id, address) | ||
.unwrap(); |
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 not sure, so I'm asking a question. If the query result is not Ok, how does the flow after unwrap?
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 the current cosmwasm, the out of gas message should be returned.
Since this is a sample contract, I don't care about it, but I should split the process as follows, depending on whether the result is Ok or not.
packages/ext/src/querier_token.rs
Outdated
}, | ||
QueryTotalParam { | ||
contract_id: String, | ||
target: String, |
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.
How about making this as an enum?
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.
Ok, I changed as an enum.
* feat: Implement token wrapper library and complete token tester * fix: used enum for target param
Closes: #3
Description
This PR should be merged after #6 has been merged.
After PR#6 is merged, I'll combine the commits into one, so files such as the circleci config will be stripped out of the changed files.
Motivation and context
How has this been tested?
Checklist: