search

mastercoin-MSC / mastercore

mastercore info
mastercoin.org
MIT License
24 stars 11 forks source link

Crowdsale grants should not be adjusted based on divisibility #234

Closed dexX7 closed 9 years ago

dexX7 commented 9 years ago

1) A1 creates new crowdsale with a rate of 1 DivisibleToken for 1 TMSC (base unit, not 1.0) 2) A2 invests 5.0 TMSC and should have 5.0 DivisibleToken

That's fine, but when doing this with indivisible tokens:

1) A1 creates new crowdsale with a rate of 1 IndivisibleToken for 1 TMSC (base unit, not 1.0) 2) A2 invests 5.0 TMSC and should have 500000000 IndivisibleToken

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.

dexX7 commented 9 years ago

I suggest to address this before the release.

dexX7 commented 9 years ago

This is off by a factor of 100000000?

Hope this is more clear:

1) Create crowdsale with 0.00000001 DivisibleToken for 0.00000001 MSC

2) Create crowdsale with 1 IndivisibleToken for 0.00000001 MSC

@m21 @zathras-crypto @marv-engine

zathras-crypto commented 9 years ago

This is current behaviour in code? If so we should be well out of consensus on already processed crowdsales no?

dexX7 commented 9 years ago

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.

zathras-crypto commented 9 years ago

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.

zathras-crypto commented 9 years ago

2) Create crowdsale with 1 IndivisibleToken for 0.00000001 MSC

Wasn't the MaidSafe crowdsale 3400 indivisible tokens for each divisible unit of prop 1 (unit being 1 MSC) ?

$ ./mastercored getcrowdsale_MP 3
{
    "name" : "MaidSafeCoin",
    "active" : false,
    "issuer" : "1ARjWDkZ7kT9fwjPrjcQyvbXDkEySzKHwu",
    "propertyiddesired" : 1,
    "tokensperunit" : "3400",
    "earlybonus" : 10,
    "percenttoissuer" : 0,
    "starttime" : 1398154020,
    "deadline" : 1400749200,
    "amountraised" : "93249.21601551",
    "tokensissued" : "452552412",
    "addedissuertokens" : "0",
    "closedearly" : true,
    "maxtokens" : false,
    "endedtime" : 1398173740,
    "closetx" : "b8864525a2eef4f76a58f33a4af50dc24461445e1a420e21bcc99a1901740e79"
}
dexX7 commented 9 years ago

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)

Creation:

gettransaction_MP 6d2989379807a009879ac63f8045568693806ca6bd456334afc39ef2d89195e8
{
    "txid" : "6d2989379807a009879ac63f8045568693806ca6bd456334afc39ef2d89195e8",
    "sendingaddress" : "mkgaNTTWVxzt8VN2BkbPCGnd3EgVabWVuF",
    "ismine" : true,
    "confirmations" : 2,
    "fee" : 0.00010000,
    "blocktime" : 1422424951,
    "version" : 0,
    "type_int" : 51,
    "type" : "Create Property - Variable",
    "propertyid" : 13,
    "propertyname" : "MDiv",
    "divisible" : true,
    "amount" : "0.00000000",
    "valid" : true
}

Invest 0.00000001 MSC:

gettransaction_MP 1264f8ed89a8cc60b6e0fd54d5b89d9575d252045a023c410f224876d4318df7
{
    "txid" : "1264f8ed89a8cc60b6e0fd54d5b89d9575d252045a023c410f224876d4318df7",
    "sendingaddress" : "n1XWoz9Q7m2bAZMRtre9EDhxpy3oK77h7v",
    "referenceaddress" : "mkgaNTTWVxzt8VN2BkbPCGnd3EgVabWVuF",
    "ismine" : true,
    "confirmations" : 1,
    "fee" : 0.00010000,
    "blocktime" : 1422424952,
    "version" : 0,
    "type_int" : 0,
    "type" : "Crowdsale Purchase",
    "propertyid" : 1,
    "divisible" : true,
    "amount" : "0.00000001",
    "purchasedpropertyid" : 13,
    "purchasedpropertyname" : "MDiv",
    "purchasedpropertydivisible" : true,
    "purchasedtokens" : "0.00000000",
    "issuertokens" : "0.00000000",
    "valid" : true
}

Crowdsale:

getcrowdsale_MP 13
{
    "name" : "MDiv",
    "active" : true,
    "issuer" : "mkgaNTTWVxzt8VN2BkbPCGnd3EgVabWVuF",
    "propertyiddesired" : 1,
    "tokensperunit" : "0.00000001",
    "earlybonus" : 0,
    "percenttoissuer" : 0,
    "starttime" : 1422424951,
    "deadline" : 7731414000,
    "amountraised" : "0.00000001",
    "tokensissued" : "0.00000000",
    "addedissuertokens" : "0.00000000"
}

getactivecrowdsales_MP
[
    {
        "propertyid" : 13,
        "name" : "MDiv",
        "issuer" : "mkgaNTTWVxzt8VN2BkbPCGnd3EgVabWVuF",
        "propertyiddesired" : 1,
        "tokensperunit" : "0.00000001",
        "earlybonus" : 0,
        "percenttoissuer" : 0,
        "starttime" : 1422424951,
        "deadline" : 7731414000
    }
]
dexX7 commented 9 years ago

3400 indivisible tokens for each divisible unit of prop 1 (unit being 1 MSC)

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?):

gettransaction_MP 2b856eb0906f4489beeb7c8a7e9a55eb8fa45305f5f2b816bebbe555f231047e
{
    "txid" : "2b856eb0906f4489beeb7c8a7e9a55eb8fa45305f5f2b816bebbe555f231047e",
    "sendingaddress" : "mpSGs1nJwztDH6AsgYqo3tCjtKRZ2bvAp1",
    "ismine" : true,
    "confirmations" : 2,
    "fee" : 0.00010000,
    "blocktime" : 1422426389,
    "version" : 0,
    "type_int" : 51,
    "type" : "Create Property - Variable",
    "propertyid" : 14,
    "propertyname" : "MIndiv",
    "divisible" : false,
    "amount" : "0",
    "valid" : true
}

gettransaction_MP 7abfebedee136c76ace889e277f9ac85e6175d88dc8661f182649f31f419663c
{
    "txid" : "7abfebedee136c76ace889e277f9ac85e6175d88dc8661f182649f31f419663c",
    "sendingaddress" : "mkyYVVakAEJhvUuydZEMugmEhZ9jXeipT7",
    "referenceaddress" : "mpSGs1nJwztDH6AsgYqo3tCjtKRZ2bvAp1",
    "ismine" : true,
    "confirmations" : 1,
    "fee" : 0.00010000,
    "blocktime" : 1422426389,
    "version" : 0,
    "type_int" : 0,
    "type" : "Crowdsale Purchase",
    "propertyid" : 1,
    "divisible" : true,
    "amount" : "0.00000001",
    "purchasedpropertyid" : 14,
    "purchasedpropertyname" : "MIndiv",
    "purchasedpropertydivisible" : false,
    "purchasedtokens" : "0",
    "issuertokens" : "0",
    "valid" : true
}

getcrowdsale_MP 14
{
    "name" : "MIndiv",
    "active" : true,
    "issuer" : "mpSGs1nJwztDH6AsgYqo3tCjtKRZ2bvAp1",
    "propertyiddesired" : 1,
    "tokensperunit" : "1",
    "earlybonus" : 0,
    "percenttoissuer" : 0,
    "starttime" : 1422426389,
    "deadline" : 7731414000,
    "amountraised" : "0.00000001",
    "tokensissued" : "0",
    "addedissuertokens" : "0"
}

When investing 1.0 MSC, the investor gets 1 Indivisible (should be: 100000000 Indivisible?):

gettransaction_MP d8124cb85530c4a5a76d05b1965ea370ac2d66baaf9e7690b789ce03e61e6bf2
{
    "txid" : "d8124cb85530c4a5a76d05b1965ea370ac2d66baaf9e7690b789ce03e61e6bf2",
    "sendingaddress" : "msR8cec6zNG6duBpZyqYo8MaPbvR6qbcex",
    "ismine" : true,
    "confirmations" : 2,
    "fee" : 0.00010000,
    "blocktime" : 1422426570,
    "version" : 0,
    "type_int" : 51,
    "type" : "Create Property - Variable",
    "propertyid" : 15,
    "propertyname" : "MIndiv",
    "divisible" : false,
    "amount" : "0",
    "valid" : true
}

gettransaction_MP 8645b6b0f24f2005a41699fe370f02de3f076f0dc85b0f4c14fb704591520479
{
    "txid" : "8645b6b0f24f2005a41699fe370f02de3f076f0dc85b0f4c14fb704591520479",
    "sendingaddress" : "n1unZQY8w1qrJJx7Qt13r68A6aQ1kehK89",
    "referenceaddress" : "msR8cec6zNG6duBpZyqYo8MaPbvR6qbcex",
    "ismine" : true,
    "confirmations" : 1,
    "fee" : 0.00010000,
    "blocktime" : 1422426570,
    "version" : 0,
    "type_int" : 0,
    "type" : "Crowdsale Purchase",
    "propertyid" : 1,
    "divisible" : true,
    "amount" : "1.00000000",
    "purchasedpropertyid" : 15,
    "purchasedpropertyname" : "MIndiv",
    "purchasedpropertydivisible" : false,
    "purchasedtokens" : "1",
    "issuertokens" : "0",
    "valid" : true
}

getcrowdsale_MP 15
{
    "name" : "MIndiv",
    "active" : true,
    "issuer" : "msR8cec6zNG6duBpZyqYo8MaPbvR6qbcex",
    "propertyiddesired" : 1,
    "tokensperunit" : "1",
    "earlybonus" : 0,
    "percenttoissuer" : 0,
    "starttime" : 1422426570,
    "deadline" : 7731414000,
    "amountraised" : "1.00000000",
    "tokensissued" : "1",
    "addedissuertokens" : "0"
}

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.

zathras-crypto commented 9 years ago

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: For divisible properties, the sending address will be credited with the number of tokens calculated as the corresponding "Number Properties per unit invested" value multiplied by the number of coins (units) specified in the Send message, plus that number of tokens multiplied by the percentage based on the "Early Bird Bonus %/Week" value, to eight decimal places.

The key thing to take away is number of coins (units) - for divisible crowdsales that value has always been interpreted as whole coins not the integer representation (which is kind of out of sync with the rest of how we do things).

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 tokensperunit for a indivisible property means a unit of 1

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 Z

dexX7 commented 9 years ago

Ah, thanks, this is roughly what I expected.

which is kind of out of sync with the rest of how we do things

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.

dexX7 commented 9 years ago

See: https://github.com/mastercoin-MSC/spec/issues/307#issuecomment-92759142