lendex-token: Cw2222 style distribution

[hashedone]

Now it is "ready ready" - with proper testing. Removed draft label.

[hashedone]

I addressed some (most?) comments, and if not addressed - answered. I don't merge yet because of one simple reason: the computation precision. In particular - do we want to be fully compliant with unsigned 128-bit storage values, or can we go away with presuming that every real live situation would fit in 96 bits? And do we just assume everything is ok, or performing every single addition with overflow check? Those are actually relevant questions and I understand @maurolacy concerns, it is just in every single contract we actually safely assume 128 bits are good enough and do not check every addition (we check substraction when we actually expect they might fail - like insufficient balance). The only (and the good reason) to take some time on this is that I actually reduced "safe" space for calculations to 96 bits. It is plenty of place, but maybe for our calculations it might not be enough. We need to agree on something here.

@ethanfrey @maurolacy @uint @ueco-jb we need some proper points and reasonable final decision here.

[maurolacy]

@ethanfrey @maurolacy @uint @ueco-jb we need some proper points and reasonable final decision here.

Merge as-is and create an issue to revisit this with 256-bit math, and/or checked / saturating ops?

I know we don't do all these checks in all contracts, but we do do it in some of them. Also, this looks like it'll never fail, until it does.

And, finally, some of these can (perhaps) be abused from the client side? If you send a u128 value greater than i128::MAX that is then cast to i128, you basically produced a negative value. That will mess with all the contract logic. Google "integer overflow exploit" for some context, and fun reading. This was how OpenBSD was hacked years ago, by the way.

Bonus: http://phrack.org/issues/60/6.html (I'm old, I know)

[ethanfrey]

And, finally, some of these can (perhaps) be abused from the client side? If you send a u128 value greater than i128::MAX that is then cast to i128, you basically produced a negative value. That will mess with all the contract logic. Google "integer overflow exploit" for some context, and fun reading. This was how OpenBSD was hacked years ago, by the way.

This is highly unlikely, but could be revisited. The others basically panic if you try to put too many tokens in. This could actually corrupt the data somehow (but I see no feasible way to do that on a real token... and we must whitelist the tokens before the market... beware of some token with total supply of u128::max.)

That said, using a safe conversion from u128 to i128 (that errors/panics if invalid) would be nice, and a good piece of a cw standard math lib

[hashedone]

I don't believe in possibility to abuse here. The tokens are not just "set" they are increased by transfers. To abuse this mechanic, user would have to be able to transfer amount of tokens which have actually 128 bit representation. He have to own such amount of tokens. If we find the place where there exists single token of this quantity, we should already have all contracts migrated to 256 bits or bigger representation.

[hashedone]

We do. If you want to, please use cosmwasm_std::Uint256. Or you mean, they don't actually support most math operations?

It does not support modulo which is critical for this usecase.

[hashedone]

Safe enough for me. And on overflow the contract panics...

No it does not. They do only in debug mode. See: https://play.rust-lang.org/?version=stable&mode=release&edition=2021. Panics in debug, gives back 4 in release.

[webmaster128]

It does not support modulo which is critical for this usecase.

Let's add it!

[webmaster128]

No it does not. They do only in debug mode. See: https://play.rust-lang.org/?version=stable&mode=release&edition=2021. Panics in debug, gives back 4 in release.

This is why all contracts have

[profile.release]
opt-level = 3
debug = false
rpath = false
lto = true
debug-assertions = false
codegen-units = 1
panic = 'abort'
incremental = false
overflow-checks = true

[hashedone]

This is why all contracts have

Ok, I missed this config, will keep in mind. In this case bound checks are even less relevant here - if at some point we would overflow somewhere contract would start to panic and we would just need to migrate it to handle wider data.

[hashedone]

Let's add it!

Sure - but not as a part of this very PR. I would prefer to leave follow up task opened here than having additional math boilerplate...