OmniLayer / spec

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

Crowdsale deadline field needs a maximum allowable value #230

Open marv-engine opened 10 years ago

marv-engine commented 10 years ago

There is no limit currently, and Omniwallet was specifying the deadline in milliseconds, but the spec interprets the value in seconds, so the deadline looks like it's thousands of years in the future.

What's a reasonable limit, e.g. 1/5/10 years in the future?

It's even more important to have this value be correct because the early bird bonus percentage is based on the number of weeks from the crowdsale start to the deadline. In one test case, I ended up with a bonus that appeared to be hundreds of thousands times the base value. (This would be avoided if the crowdsale tx specified the initial early bird bonus percentage directly rather than it being per week.)

There may be other situations where we should have a "reasonable" limit imposed on input values.

marv-engine commented 10 years ago

Because there's no max value for the deadline in the spec, the protocol has to accept any value that's interpreted to be in the future, regardless of how far in the future.

Related - the benefit of changing the early bird bonus % per week to be an initial early bird bonus % is that the bonus could never be greater than the value in the tx message field (255% max). The worst that would happen is that the bonus percentage would decrease to zero from the initial value over thousands of years.

The early bird bonus % has no limit currently because it's computed based on the number of weeks between the start and the deadline. If the deadline is thousands of years in the future (as it can be now), the bonus percentage will be 52 * (thousands of years) * (early bird bonus % per week). No one will be happy with the huge number of tokens issued in that case.

dexX7 commented 10 years ago

Sidenote: Bitcoin's time value field in blocks is only 32 bit wide and not 64 bit as the deadline field.

dacoinminster commented 10 years ago

I'd suggest we limit this in the UI rather than the spec, but I'm not opposed to a spec limitation if that is strongly desired. We shouldn't make any changes that would invalidate prior crowdsales.

marv-engine commented 10 years ago

We can't rely on the UI to enforce reasonableness, especially when there are more 3rd party front ends using Master Core's API. It's unfortunate that there are crowdsales with outrageous end dates - due to incomplete testing.

We can be backwards compatible for the crowdsale deadline, at least, by checking old tx's for a value greater than X (some reasonable date time). New tx's (after some block number) would have to meet the reasonableness test.

dacoinminster commented 10 years ago

So, let's look at a crowdsale that would fail a reasonableness test: Imagine somebody decides they want to do a 200 year crowdsale. Sure, it seems strange, but why should we try to stop them? Are we worried about user typos? Or are we concerned about processing bandwidth on our parsers?

genecyber commented 10 years ago

@marv-engine if it makes you feel any better, the sale that's mine I like having the crazy end date on it. I've got a crowdsale that only accepts tokens of a particular type. only 1 of these tokens happen to be in existence. And the issuance address can send the token to themselves. This creates an on demand token generator. One that no one else can use and will last ~forever.

marv-engine commented 10 years ago

@dacoinminster given the way the early bird bonus percentage is calculated, a date far in the future can result in a gargantuan early bird bonus percentage. Worst case is 255 * 52 weeks * 1000's of years. The fact that there's no easy Undo mechanism, I'm very cautious about letting this happen.

dacoinminster commented 10 years ago

Yup - you can't undo - only start over.

The UI can make it very clear what is happening though.

genecyber commented 10 years ago

If we go with a max can we also get an ∞ that will void the bonus that's a problem? 

An infinite sale that accepts a particular sp is how I did an issuance. The ability to store the token type somewhere is a nice insurance. (can always make more) 

And the on demand aspect mimics the counterparty style "on demand" issuance. 

Oh.... I just noticed the new issuance type PR makes my use case a lousy hack :)

On Fri, Aug 29, 2014 at 12:49 PM, dacoinminster notifications@github.com wrote:

Yup - you can't undo - only start over.

The UI can make it very clear what is happening though.

Reply to this email directly or view it on GitHub: https://github.com/mastercoin-MSC/spec/issues/230#issuecomment-53901465

dexX7 commented 9 years ago

Inbefore: sorry for the OT.

I've got a crowdsale that only accepts tokens of a particular type. only 1 of these tokens happen to be in existence. And the issuance address can send the token to themselves. This creates an on demand token generator. One that no one else can use and will last ~forever.

You are awesome. I might miss something, but this makes Tx 56: Granting Tokens for a Managed Property completely - or at least partially - obsolete? (effectiveness and usability aside)

Edit: a generator could also be used for a limited and faulty transfer of asset ownership where generator tokens are transferred instead of "ownership" itself.

genecyber commented 9 years ago

Haha yes look at my Prozcoin issuance on mainnet.

On Fri, Oct 3, 2014 at 11:26 PM, dexX7 notifications@github.com wrote:

Inbefore: sorry for the OT.

I've got a crowdsale that only accepts tokens of a particular type. only 1 of these tokens happen to be in existence. And the issuance address can send the token to themselves. This creates an on demand token generator. One that no one else can use and will last ~forever.

You are awesome. I might miss something, but this makes Tx 56: Granting Tokens for a Managed Property completely - or at least partially - obsolete? (effectiveness and usability aside)

Reply to this email directly or view it on GitHub: https://github.com/mastercoin-MSC/spec/issues/230#issuecomment-57893329