Use blocktime to specify when an item in the spec takes effect

[marv-engine]

Using a block number as we do now is imprecise and it requires some last minute estimation to try to get close to the desired time. If we're shooting for a time why use a block number?

Using blocktime eliminates the error window and it can be set far in advance. It's also easier for humans to remember a timestamp rather than a blocknumber. I believe bitcoin client code uses blocktime for this purpose.

[dacoinminster]

Because blocktime can be just as sketchy (or maybe even worse). For
instance, we found at that blocktime can go backwards briefly! Obviously
we'd use the first blocktime which crossed the threshold, and ignore
earlier times in later blocks, but that's painful to explain. To avoid such
ambiguity questions, I think we decided to stick with block number in an
earlier conversation.

On Tue, Sep 23, 2014 at 6:53 AM, Marv Schneider notifications@github.com
wrote:

Using a block number as we do now is imprecise and it requires some last
minute estimation to try to get close to the desired time. If we're
shooting for a time why use a block number?

Using blocktime eliminates the error window and it can be set far in
advance. It's also easier for humans to remember a timestamp rather than a
blocknumber. I believe bitcoin client code uses blocktime for this purpose.

—
Reply to this email directly or view it on GitHub
#260.

[dexX7]

Block time is ambigious, because timestamps do not necessarily appear in order and Bitcoin time might as well go backwards - within some range. The block height on the other hand is guarantueed to be +1 each time (reorganizations aside).

This fuzziness is actually the reason why I'd prefer setting the deadline of a crowdsale in terms of block height as well and @dacoinminster already hinted the "painful" way how it's handled at the moment.

Related: https://groups.google.com/a/mastercoin.org/d/msg/dev/33f03j5g05M/PaGgfg6x4OsJ

[marv-engine]

Using block number requires last minute updating of the spec and any code that's based on it. Two suggestions:

1. the spec should be updated retroactively with the blocktime associated with that block number. That will make these functionality changes more readable.
2. MasterCore and other clients should report/log what transaction types (and versions) are supported. This will provide a real-time indication that the code is behaving as expected.

[dacoinminster]

1 sounds fine

@zathras-crypto / @faizkhan00 / @m21 should probably decide how complex 2 is vs it's utility

[ghost]

For the most part they are documented Marv, see this link for the exact code defines that specify the active block height and active transaction types (note that 999999 is basically a way to say 'disabled in mainnet' - Transactions are always enabled in Testnet and Regtest mode)

https://github.com/mastercoin-MSC/mastercore/blob/0.0.7/src/mastercore.h#L52-L95

// Transaction types, from the spec
enum TransactionType {
  MSC_TYPE_SIMPLE_SEND              =  0,
  MSC_TYPE_RESTRICTED_SEND          =  2,
  MSC_TYPE_SEND_TO_OWNERS           =  3,
  MSC_TYPE_AUTOMATIC_DISPENSARY     = 15,
  MSC_TYPE_TRADE_OFFER              = 20,
  MSC_TYPE_METADEX                  = 21,
  MSC_TYPE_ACCEPT_OFFER_BTC         = 22,
  MSC_TYPE_OFFER_ACCEPT_A_BET       = 40,
  MSC_TYPE_CREATE_PROPERTY_FIXED    = 50,
  MSC_TYPE_CREATE_PROPERTY_VARIABLE = 51,
  MSC_TYPE_PROMOTE_PROPERTY         = 52,
  MSC_TYPE_CLOSE_CROWDSALE          = 53,
  MSC_TYPE_CREATE_PROPERTY_MANUAL   = 54,
  MSC_TYPE_GRANT_PROPERTY_TOKENS    = 55,
  MSC_TYPE_REVOKE_PROPERTY_TOKENS   = 56,
};


// block height (MainNet) with which the corresponding transaction is considered valid, per spec
enum BLOCKHEIGHTRESTRICTIONS {
// starting block for parsing on TestNet
  START_TESTNET_BLOCK=263000,
  START_REGTEST_BLOCK=5,
  MONEYMAN_REGTEST_BLOCK= 101, // new address to assign MSC & TMSC on RegTest
  MONEYMAN_TESTNET_BLOCK= 270775, // new address to assign MSC & TMSC on TestNet
  POST_EXODUS_BLOCK = 255366,
  MSC_DEX_BLOCK     = 290630,
  MSC_SP_BLOCK      = 297110,
  GENESIS_BLOCK     = 249498,
  LAST_EXODUS_BLOCK = 255365,
  MSC_STO_BLOCK     = 999999,
  MSC_METADEX_BLOCK = 999999,
  MSC_BET_BLOCK     = 999999,
  MSC_MANUALSP_BLOCK = 999999,
  P2SH_BLOCK        = 999999,
};

[dexX7]

@faizkhan00: this is not sufficient imho. I mean.. even Michael assumed tx 51 v1 "has been live for months" until zathras unveiled the implementation hasn't even started. (edit: but that aside: there is also a difference between tx n version 0 and version 1 in some cases which is not covered by this list)

It would be neat, if there were a detailed overview of:

mastercore version - transaction type - transaction version - status mainnet - status testnet

Or similar... once that's done, the spec could be cleaned.

Especially tx 51 is a chaos right now, because it's not even clear which parts belong to v0 and v1 (imho) and I still have no clue, how a proper v1 transaction would look like, see: #249.

And also very related:
mastercoin-MSC/mastercore#118 (comment)
mastercoin-MSC/mastercore#118 (comment) (list of spec commits that are ready for review/merge)

Final goal:
#188 Revision control, start to tag spec releases (...)

[m21]

In addition to the tables by Faiz here's the one that actually attempts to deal with V0 vs V1 logic:
// TODO: there would be a block height for each TX version -- rework together with BLOCKHEIGHTRESTRICTIONS above
static const int txRestrictionsRules[][3] = {
{MSC_TYPE_SIMPLE_SEND, GENESIS_BLOCK, MP_TX_PKT_V0},
{MSC_TYPE_TRADE_OFFER, MSC_DEX_BLOCK, MP_TX_PKT_V1},
{MSC_TYPE_ACCEPT_OFFER_BTC, MSC_DEX_BLOCK, MP_TX_PKT_V0},
{MSC_TYPE_CREATE_PROPERTY_FIXED, MSC_SP_BLOCK, MP_TX_PKT_V0},
{MSC_TYPE_CREATE_PROPERTY_VARIABLE, MSC_SP_BLOCK, MP_TX_PKT_V1},
{MSC_TYPE_CLOSE_CROWDSALE, MSC_SP_BLOCK, MP_TX_PKT_V0},
{MSC_TYPE_SEND_TO_OWNERS, MSC_STO_BLOCK, MP_TX_PKT_V0},
{MSC_TYPE_METADEX, MSC_METADEX_BLOCK, MP_TX_PKT_V0},
{MSC_TYPE_OFFER_ACCEPT_A_BET, MSC_BET_BLOCK, MP_TX_PKT_V0},
{MSC_TYPE_CREATE_PROPERTY_MANUAL, MSC_MANUALSP_BLOCK, MP_TX_PKT_V0},
{MSC_TYPE_GRANT_PROPERTY_TOKENS, MSC_MANUALSP_BLOCK, MP_TX_PKT_V0},
{MSC_TYPE_REVOKE_PROPERTY_TOKENS, MSC_MANUALSP_BLOCK, MP_TX_PKT_V0},

Note the "TODO" -- I haven't reworked it yet, not sure if the spec got it all easily shown somewhere. :)

Reporting/logging what is supported is easy to do (dump the above 2-3 tables into the log each time we run, etc.).
But should determine whether the information we have in MasterCore is complete (or I have to actually take care of the TODO above first).

[m21]

On BlockTime vs BlockNumber I am 100% for BlockNumber as it's very deterministic, does not roll back, can be reasonably estimated, etc.
BlockTime is something each miner puts in based on whatever local clock they got, which can drift freely.

[dexX7]

Ah, this list is more clear. Only flaw:

{MSC_TYPE_CREATE_PROPERTY_VARIABLE, MSC_SP_BLOCK, MP_TX_PKT_V1},

v1 is not implemented - or maybe it's a bastardized (literally, not meant as offense ;) version of v0 and v1.

The spec is currently unclear about this point and if I'm not mistaken, the following changes were made:

• one crowdsale per ecosystem per address (v0) vs. one crowdsale per address (v1)
• crowdsale does not end, if MAX_INT64 units are sold (v0) vs. crowdsale ends, if this is true (v1)
• different bonus/percentage calculations (IIRC)
• bitcoin is not allowed (v0) vs. bitcoin is allowed (v1)
• only one currency can be accepted (v0) vs. multiple currencies can be accepted (v1)

---

Re: block height: +1

I once suggested - for the sake of keeping the timestamp - to mirror the behavior of nLockTime:

If value < 500000000 it's block height, if value >= 500000000 it's unix timestamp. (FYI: 50..0 <=> 05.11.1985 - 01:53:20)

[marv-engine]

@m21

Reporting/logging what is supported is easy to do (dump the above 2-3 tables into the log each time we run, etc.).

That's what I had in mind. The code reports what tx's it supports when it starts.

[msgilligan]

@marv Now that we have the OLE process, once an OLE gets final approval, an "effective at block height" field should be set in the OLE header.

[msgilligan]

@marvgmail @dexX7 @achamely @CraigSellars I think we should close this issue.