Treasury #NTRN-286 #9
Conversation
|
@ratik good job! I would like to approve your PR, but there are still three threads open from my side, let's resolve them? |
contracts/treasury/src/msg.rs
Outdated
| pub owner: String, | ||
| pub denom: String, | ||
| /// Distribution rate in percent (0-100) which goes to distribution contract | ||
| pub distribution_rate: u8, |
There was a problem hiding this comment.
Why not Decimal type? It's easier to do some math stuff
There was a problem hiding this comment.
Dunno, just matter of taste. As for me 30(%) looks more readable than 0.3
There was a problem hiding this comment.
but if you insist I'm happy to change it
There was a problem hiding this comment.
I'd change it to Decimal because:
Decimalis commonly used for someratevariables in cw contracts, that's why it was my first question for the PR :)- built-in math methods for percentage calculations;
more readable (but not for you 🙃 )
Wdyt?
There was a problem hiding this comment.
ok, np. changed to Decimal
There was a problem hiding this comment.
Would be nice to have comments for some core methods of contracts: for example execute_fund in the distribution contract would be easier to read if the method was covered with comments.
The same about constants in state.rs files: I think it's better to have a small description for such constants.
https://p2pvalidator.atlassian.net/browse/NTRN-286
Integration tests: neutron-org/neutron-integration-tests#49