-
Notifications
You must be signed in to change notification settings - Fork 118
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
Tx51 (crowdsale) version 1 accepts multiple currencies, including bitcoins #185
Conversation
We need to specify the block number where tx51 version 1 will be turned on. * tx51 version 1 allows 0 (bitcoin) for Currency Identifier desired * early bird bonus no longer tied to weeks * number of tokens issued limited by max Number of Coins value * Number of Coins now a signed 8-byte integer
|
||
percentage = (("Deadline" value in seconds - transaction timestamp in seconds) / 604800) * "Early bird bonus %/week" value | ||
(1 + (percentage / 100.)) * "Number Properties per Unit Invested" value * the number of coins sent by the purchaser |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"100."? What's the point used for there? If it is, what I think, then I suggest to use "100.0", "100f" or similar instead. It's not all that clear, if this is simply a typo or intended.
Just to make sure: the way the bonus is calculated is fundamentally different now? Say for example I wanted to create a crowdsale with a bonus structure similar to the Exodus funding. In version 0 I'd set the deadline to four weeks in the future and the bonus to 10 %. Correct? |
@dexX7 you are correct regarding calculation of bonus. |
@dexX7 I can change 100. to 100.0 My goal is for the implementations to use enough precision (double precision?) for all calculations and then convert to the target data type after the calculation is complete. The reason to remove any reference to weeks for the early bird bonus is that it was confusing the way it was written. People thought the bonus declined as a weekly step function, not linearly by the second as it does. Also, the week reference was unclear about how partial weeks are handled (e.g. 30 days vs 28). Version 1 is intended to eliminate the confusion and make it easy to use. |
Yup, agreed. This way it's more intuitive. I think we should define percision spec wide - this is also relevant for Dev MSC and DEx. Double percision should be fine. |
Is there any worry that using double precision floats will lead to floating point arithmetic issues? |
I've looked through these changes. Still needs updates due to recent breakthough regarding TX51. (multiple currencies going at once, or updates to exchange rates). I assume that's still coming (unless maybe I missed it) |
@dacoinminster yes. i was intending to do that PR after this one, but I can combine them if you want. |
Either way is fine. |
ok, i'll add it to this one. |
Re: enabling a crowdsale to accept multiple currencies by using multiple tx51's -
That sentence is insufficient because an address could have 2 active crowdsales - 1 in each ecosystem - with the same currency id desired. The protocol wouldn't know which crowdsale a purchaser is participating in. I doubt that anyone will initiate cross-ecosystem crowdsales, but the protocol has to handle it if it's allowed. I see two ways to address this:
So, any preference or objection to item 2 or 3? I like 3 because it's an additional way to keep the ecosystems from cross-pollenating. |
Um, just prevent two crowdsales from the same address even in different ecosystems. Much simpler |
Easy enough - just remove "per ecosystem" from the existing sentence. This change will apply to version 1 of tx51. Is it safe to assume that no one has tried, or will try, to exercise this case with version 0 of tx51? The spec didn't address this case. I don't know if any client detected this case, and if so, what the code did. |
When accepting multiple currencies, can the originating tx51 be updated to zero for the Number Properties per Unit Invested value? I think the issuer has to be able to stop accepting any of the currencies, including the original one. So, subsequent tx51's can stop accepting currencies as long as there's still one currency being accepted? Or, can an issuer effectively suspend temporarily all participation by setting Number Properties per Unit Invested value to zero for each of the currencies he's accepting? |
Yes, they can set the original one to zero. Yes, they can set all of them If the new TX51 has a different end date, premine, early bird bonus, name, On Thu, Jun 5, 2014 at 3:43 PM, Marv Schneider notifications@github.com
|
I understood that the same end date (Deadline) is what indicates that a new tx51 applies to the active crowdsale (and either adds a desired currency or modifies the terms for an existing one), and a different end date indicates an attempt to create a concurrent crowdsale - which is not allowed. I agree that the fields other than Currency Identifier Desired and Number Properties per Unit Invested are ignored for tx51's after the original one for an active crowdsale. We still need to address my question a few comments back in #185 (comment) |
Good move on allowing the issuer to accept multiple currency for crowd funding. I'm not sure how it works but here is my input. A new tx 51 that is sent before the crowd funding is closed or has ended means it is a "Add new currency for current crowd funding". (If it is sent after a tx 51 is closed or ended, it is a new SP) Ex. Alice sends a second tx51 Alice sends a third tx51 Creates a crowd funding that accepts Bitcoins, MSC, or MAIDSAFE. If the "Currency Identifier Desired" already exist in the previous tx51s, the tx is invalid. invalid because MSC is already defined in Second tx51. (This means an Issuer has only one chance to set the Number Properties per coin ) |
tweaked the wording to account for a crowdsale accepting multiple currencies.
I haven't reviewed the new version of this, but @Bitoy: the option to edit the number of properties credited for token X on the fly is there to adjust to changing exchange rates. |
@Bitoy, @dexX7 Yes, the number of tokens issued per unit sent can be updated multiple times during the crowdsale on a currency-by-currency basis. If it's set to 0, then the crowdsale is not accepting that currency, either temporarily or permanently. This applies to any currency accepted by the crowdsale, including the original one. It's up to the issuer to prep for/deal with participant reaction to changes in the issue rate. |
General rant ;).. inconsistent usage of case sensitivity (we should really push a general document related to address terminology, but given that I'm no native English speaker, I don't feel appropriate for this role). "tx51" may be replaced by the full identifier. Also slightly related: |
From another user:
I assume this is done as usual? See: #131 Edit: It was suggested to explicitly include an exchange rate in the payment transaction for the case that an user sends coins/tokens and the crowdsale terms changed in the meantime. |
Sure - they can suspend all if they want |
@marv-engine ok I now understand why Updating the no. Of properties per coin is left to the issuer. Crowd sale v 1 looks ready. ( Can't wait to see it in action :) |
This issue still needs to be addressed:
So,
How do we prevent this spec bug from being exercised by a user? |
In response to the most recent comment from @dexX7 #185 (comment) My expectation, based on #131 - A tx51 that changes a crowdsale affects any sends that come after that tx51, regardless of which block(s) the sends are in.
Simple Send and bitcoin Send messages don't have a way to include an exchange rate. (I do have some ideas that should work for Simple Send without manual intervention, but wouldn't work for bitcoin Send.) It's left to the issuer to respond to requests for some kind of adjustment if the purchaser doesn't like the exchange rate he ended up with or if the crowdsale closed before the payment was processed. I wouldn't want to be either party in a case involving a large payment. |
Simple sends could be enhanced by additional data (exchange rates) and something simliar could be attached to Bitcoin payments -- but I really don't like it. I agree, this is a topic which issuers should address. In most cases I assume the issuer likes to act proactive anyway. Edit: there could be a grace periode. For example a change takes effect three blocks after the first confirmation. Same for closing a crowdsale. I suggested this at some other place earlier (#132) and I don't really like it either. I'd rather see, where this is going and if it evolves to be a problem. |
@marv-engine Assuming the user creates 2 crowd sales. User that send " test msc" coins goes to crowd sale Eco 2 There is no conflict. |
But Test MSC can be sold for BTC? |
Ah ok thanks @dexX7. The dex tx 21 doesn't have an ecosystem property so it's valid to sell test msc for bitcoins. ( just trying to fix the conflict :) |
The ecosystem flag does nothing else than creating trouble.. ;) :P @Bitoy: tx 20 (not 21!*) uses currency ids which imply the ecosystem. TMSC are a special case related to the id and ecosystem. See: Field: Currency identifier So in a strict interpretation: MSC is ecosystem 1, TMSC is ecosystem 2. Therefore, based on tx 20, Bitcoin trades are allowed in both ecosystems.
I didn't see it at first, but there is acutally another restriction. It's not attached to the transaction and only loosely written in the general description for smart properties:
Tx 51 is related to the creation of a crowdsale and if we decide to limit version 1 to allow only one crowdsale overall, then the conflict related for Bitcoin seems to be solved, because even if a user goes ahead and creates a Bitcoin nominated crowdsale in whatever ecosystem with tx51v1 and then uses tx51v0 to add another one, the second one must be in a different ecosystem (as required by v0) and since v0 can't use Bitcoin, there is no case where a user might create two crowdsales with Bitcoin? Edit: if the limitation of one active crowdsale in v1 is applied, this also implies changing a crowdsale via tx51v1 can only be used, if there is only one, and not two, active crowdsales. *How does tx 21 (sell token for token) address trades between ecosystems? Something that may be considered as restriction is also written in the general "smart property" description:
Is this applied? I strongly suggest we don't set rules outside of the direct scope of transactions - statements like this have no version number. |
Guys, no exchange rates in simple sends. Too complicated. This would become an "investment send". Which is something we can always add later It's up to the issuer to fix any problems caused by their monkeying with exchange rates! |
@dacoinminster we're not looking to add exchange rates to simple sends. But, please look at my #185 (comment) |
from a conversation yesterday, @dacoinminster says to disable tx51 version 0 starting with the same block that version 1 is enabled. |
Fine. I hope there will some kind of announcement? FYI: Tx20v0 was nerver disabled. So only one active crowdsale, yes? Should I open a new issue to address inter-ecosystem trading via tx21? |
@dexX7 as you noted in an earlier comment, there is no exposure with tx21v0, so technically we don't need to disable it. Thanks for that catch, from the spec:
Yes, please create another issue to discuss tx21 (sell one MSC-based currency for another MSC-base currency). This PR is about tx51 (crowdsale). |
Tx51 (crowdsale) version 1 accepts multiple currencies, including bitcoins
We need to specify the block number where tx51 version 1 will be turned on.