-
Notifications
You must be signed in to change notification settings - Fork 11
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
Crowdsale grants should not be adjusted based on divisibility #234
Comments
I suggest to address this before the release. |
This is off by a factor of 100000000? Hope this is more clear:
|
This is current behaviour in code? If so we should be well out of consensus on already processed crowdsales no? |
Yes, current behavior. Given that this issue is already open for a while, I guess this is not new however and it's even longer ago that crowdsale logic was touched at all. I'm still surpirsed. |
Are there some test results for this? Don't suppose it's a spock test is it by any chance? Kind of beggars belief that crowdsales crossing divisibility would be off by a factor of 10000000 and that sounds like we should be pulling the tag to be honest and sorting this out :( Either that or a fix has to wait for our 0.0.10 release (MetaDEx) because it'll be a consensus affecting change. |
Wasn't the MaidSafe crowdsale 3400 indivisible tokens for each divisible unit of prop 1 (unit being 1 MSC) ?
|
I stumbled upon this issue earlier and started with the Spock tests to very the results. I'm out of time, but I'll try to get some verbosed output soon. Case 1)
|
This is what this issue is about actually. 1.0 MSC is 100000000 "units", and this is why "1 IndivisibleToken for 0.00000001 MSC" doesn't work: Case 2) Create crowdsale with 1 IndivisibleToken for 0.00000001 MSC When investing 0.00000001 MSC, the investor gets nothing (should be: 1 Indivisible?):
When investing 1.0 MSC, the investor gets 1 Indivisible (should be: 100000000 Indivisible?):
Edit: it's probably now to late before the release, and I wouldn't delay any further.. it's not nice, but it's bahavior that is live since the beginning (probably), so there doesn't seem to be a hurry. |
Ah :) OK I feel a bit better now as I think we have an issue of semantics here rather than code (and perhaps a poorly worded spec? I'll check it out). Long story short, from spec: The key thing to take away is So tl:dr; when you see "tokensperunit" when requesting a divisible property to fund a crowdsale, the unit refers to a whole token (1.00000000). (Edited sorry poorly worded) Never really given it a second thought to be honest, this is how it's always been done. tokensperunit for a divisible property means a unit of 1.00000000 So I don't think we have so much of a coding issue as more of a implementation approach/methodology discussion but for which the behaviour has been fixed like this since the early days (as far back as MaidSafe crowdsale at least). FYI 'Chest implemented the same methodology, as did msc-tools I believe. Does that help explain at all? Thanks |
Ah, thanks, this is roughly what I expected.
Yup, everything else (?) is without "amount amplification". As consequence, the maximum amount that could be crowdsaled is 92233720368 Indivisible for 92233720368.0 Divisible, if I'm not mistaken, whereby the number of indivisible tokens that can exist caps at 9223372036854775807 Indivisible. Well, at least for BTC crowdsales I see some value in adjusting the ranges. |
That's fine, but when doing this with indivisible tokens:
But A2 has only 5. Looks like it's being adjusted based on divisibility. Likewise, sending 0.1 TMSC results in a grant of 0 IndivisibleToken etc.
If I'm not totally mistaken, than that's unexpected behavior.
The text was updated successfully, but these errors were encountered: