OmniLayer / spec

Omni Protocol Specification (formerly Mastercoin)
The Unlicense
342 stars 116 forks source link

Clarify differences between Divisible and Indivisible in the Spec. #307

Open msgilligan opened 9 years ago

msgilligan commented 9 years ago

The specification currently says:

The difference between divisible and indivisible property types is how they are displayed…

It doesn't say "the only difference" is how they are displayed. Nor does it explicitly say that all internal computations are the same for divisible and indivisible tokens.

If this is true the specification should be updated to say so. If not, the specification should clarify the differences in computation required between the two.

msgilligan commented 9 years ago

Issue #210 is related. If a UI wants to place the decimal somewhere else, are there any issues with doing that? Bitcoin Core and Omniwallet for Windows both allow users to switch between BTC, mBTC, etc. and, if I recall correctly, the consensus in the Bitcoin community is that Bitcoin should move to the preferred units of 'bits' which are 100 satoshis, or two decimal places.

dexX7 commented 9 years ago

@msgilligan: there is an exception (which I consider as bug). Check out this thread: https://github.com/mastercoin-MSC/mastercore/issues/234

Edit: related test (fails, IIRC): https://github.com/msgilligan/OmniJ/compare/master...dexX7:crowdsale-1-to-1

(This time in this thread.. seems more correct... :)

zathras-crypto commented 9 years ago

@dexX7 I disagree this is a bug (bug implies error/flaw/failure but the code works as intended).

It's a semantic discussion on what "tokens per unit" refers to, but the rules applied in code are the same rules that have applied to crowdsales since the very beginning. Changing them requires not a bug fix but a new version of the crowdsale message.

Note, I don't see your alternative of using the "unit" in tokens per unit as 0.00000001 instead of 1.00000000 for divisibles works mate - let's take the MaidSafe crowdsale - their intention was to transfer 3400 indivisible tokens per MSC invested (let's ignore the bonus for simplicity).

Current (interpret as 1.0000000): "tokensperunit" set at 3400. User gets 3400 tokens per MSC. Alternative (interpret as 0.00000001): "tokensperunit" needs to be set at 0.00003400. User gets 3400 tokens per MSC.

The alternative case is impossible because 0.00003400 is an invalid indivisible amount for the tokens per unit field. Thus the exampled crowdsale could not have been run unless the rules were applied as they currently are.

More than happy to flesh this out some more bud, but it's important to remember a couple of things:

zathras-crypto commented 9 years ago

Put another way, crowdsale me 5 indivisible tokens per MSC where tokensperunit can only be indivisible and we interpet "per unit" to mean 0.00000001 instead of 1.00000000 :)

dexX7 commented 9 years ago

the rules applied in code are the same rules that have applied to crowdsales since the very beginning

I'm aware. I linked the post only as reference, in the context of "can we ignore divisibility altogether, and are there special cases to consider".

It's a semantic discussion on what "tokens per unit" refers to

Thanks @zathras-crypto. I have to think about this (I'm a bit unfocused at the moment), and if that is compatible with a model, where divisibility doesn't exist per se, but only "base units".

dexX7 commented 9 years ago

The alternative case is impossible ...

Yes, this is correct.

It's a semantic discussion on what "tokens per unit" refers to ...

This is really what this is about, I'm sort of convinced, and I'm going to accept this statement. Thanks again for putting it in another words. :)