-
Notifications
You must be signed in to change notification settings - Fork 353
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
Implement airdrop start expire #608
Conversation
371678c
to
0dfcc1a
Compare
0dfcc1a
to
539eb5b
Compare
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.
Looks good.
Please rebase now that #606 was merged, so this makes a cleaner diff.
I made a small style suggestion but the code looks correct, so it is optional.
info: MessageInfo, | ||
stage: u8, | ||
amount: Uint128, | ||
proof: Vec<String>, | ||
) -> Result<Response, ContractError> { | ||
// airdrop begun |
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 would pull out these lines (the green diff) into a function like:
is_active(deps: Deps, env: Env, stage: u8) -> StdResult<bool> {}
which you can call here and easily unit test. You may well want to use the same check somewhere else later.
}; | ||
execute(deps.as_mut(), env.clone(), info.clone(), msg).unwrap(); | ||
|
||
// can't claim expired |
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.
comment needs updating "can't claim before it starts"
Please move to https://github.com/CosmWasm/cw-tokens You cannot transfer PR. You may be able to cherry-pick commits if you are a git super-star. Or simply apply the patch. I kept the same contract names and paths. |
Blocked by #606
Useful feature.