Various cleanup, reindexing and resolving LOCK issue

[dexX7]

Notable changes:

• Removal of unused, or old code, which very likely isn't going to be used anymore
• Formatting, restructuring here and there
• Moving code from headers into cpp files
• Split alert/notification code
• Enable block disconnect handlers during reindex, and wipe DBs during reindex

Alert related:

• Authorize mainnet address: 1dexX7zmPen1yBz2H9ZF62AK5TGGqGTZH (resolves Grant @dexx7 alert rights #53)
• Authorize testnet address: mk5SSx4kdexENHzLxk9FLhQdbbBexHUFTW (resolves Grant @dexx7 alert rights #53)
• Alert payloads can be created via CreatePayload_OmniCoreAlert()
• There is a hidden RPC call "sendalert_OMNI" to create alerts
• "-alertignoresender=address" can be used to backlist specific alert senders
• "-alertallowsender=address" can be used to whitelist specific alert senders
• "-alertallowsender=any" can be used to consider any source as authorized (for tests!)

Fixes:

• Resolve wallet LOCK issue
• Only create class C transactions, if they are also accepted (resolves Wallet creates class C transactions, even if not enabled yet #69)
• Extend marker check to cover alerts as well (resolves Class C encoded alerts don't pass the marker check #70)
• Fix height check of CMPTxList::setLastAlert()

[dexX7]

Updated. The following items were added:

Alert related:

• Authorize mainnet address: 1dexX7zmPen1yBz2H9ZF62AK5TGGqGTZH (mine)
• Authorize testnet address: mk5SSx4kdexENHzLxk9FLhQdbbBexHUFTW (mine)
• Alert payloads can be created via CreatePayload_OmniCoreAlert()
• There is a hidden RPC call "sendalert_OMNI" to create alerts
• "-alertignoresender=address" can be used to backlist specific alert senders
• "-alertallowsender=address" can be used to whitelist specific alert senders
• "-alertallowsender=any" can be used to consider any source as authorized (for tests!)

The alert code is tested via unit tests (nearly) as far as possible.

Fixes:

• Extend marker check to cover alerts as well
• Fix height check of CMPTxList::setLastAlert()
• Only create class C transactions, if they are also accepted

Currently the payloads for alerts are pretty different from the other transaction types, and instead of using/parsing fields, it's basically a single string, which is later splitted.

This is something we might change at some point in my opinion.

[zathras-crypto]

Reviewed - more great work - with HUGE thanks @dexX7 :)

Supported, merge when you're ready :)

[dexX7]

Thanks for the fast review! :)

A few quick questions:

In CheckExpiredAlerts() (case 4)

bool txLive = (chainActive.Height()>(int64_t) expiryValue);

Was chainActive.Height() intentionally used, or should it be curBlock?

Do you agree with -alertallowsender, -alertignoresender?

I'm very sure there will never be an option with this name for Bitcoin Core, but we might add an -omni before every option. On the other hand, -omnidebug and -omnilogfile are the only ones, while other options (such as -startclean) don't start with -omni either.

[zathras-crypto]

Was chainActive.Height() intentionally used, or should it be curBlock?

Well, the original intention was:
"is there a TX live on the network that the client doesn't support?"

I guess I didn't see much value to continuing to parse knowing that we would shutdown at some future stage of parsing. Eg if you're on mainnet with a synced up blockchain, but you download an old version of Omni Core, why waste the users time parsing up until curBlock > expiryValue and then shutting down, when we can shutdown as soon as we parse the alert message. Probably not too clear, let me try examples - assuming here user has a fully synced blockchain to block 400,000.

• User downloads old version of Omni Core and starts parsing. They reach 350,000 and parse an alert that shows TX n goes live at block 399,999 and they don't support it. Client shuts down there and then.
• User downloads old version of Omni Core and starts parsing. They reach 350,000 and parse an alert that shows TX n goes live at block 399,999 and they don't support it. Client continues parsing until block 399,999 and then shuts down.

The second scenario doesn't hold much value, it's just extra wasted time spent parsing since the end result is still the same (client can't be used).

I hope that makes sense :) I'm not super fussed though so if you have spotted a reason to use curBlock instead just shout I don't feel strongly about it.

I'm very sure there will never be an option with this name for Bitcoin Core, but we might add an -omni before every option

Yeah that's an interesting Q - I think it's kind of nice to make the distinction that these startup params affect the Omni code layer only so perhaps we should standardize on making all options prefixed with -omni even if it isn't explicitly required for avoiding collisions with Bitcoin Core.

Do you agree with -alertallowsender, -alertignoresender?

Yep, though in the context of above (ie perhaps -omnialertallowsender and friends).

[dexX7]

Ah, I see. Hm.. I mean, it's pretty unrealistic that there are very, very large gaps between the alert transaction and the expiry value. I tend to prefer to use curBlock, which feels more consistent (because the value is explicitly provided), and given that we're only talking about scanning for Omni transactions, there is probably also not a very huge waste of time.

In this context: we should show a warning earlier (as soon as possible), and use a shutdown as last resort, instead doing nothing while the alert is already processed, but not yet "expired". (edit: ah, I think this is already done)

I think it's kind of nice to make the distinction that these startup params affect the Omni code layer only so perhaps we should standardize on making all options prefixed ...

Agreed.

[dexX7]

Are those email addresses correct?

// Mainnet
16Zwbujf1h3v1DotKcn9XXt1m7FZn2o4mj  Craig   <craig@omni.foundation>
1MicH2Vu4YVSvREvxW1zAx2XKo2GQomeXY  Michael <michael@omni.foundation>
1zAtHRASgdHvZDfHs6xJquMghga4eG7gy   Zathras <zathras@omni.foundation>
1EXoDusjGwvnjZUyKkxZ4UHEf77z6A5S4P  Exodus (who has access?)

// Testnet
mpDex4kSX4iscrmiEQ8fBiPoyeTH55z23j  Michael <michael@omni.foundation>
mpZATHupfCLqet5N1YL48ByCM1ZBfddbGJ  Zathras <zathras@omni.foundation>

[zathras-crypto]

I tend to prefer to use curBlock

Sure, go ahead and make the change mate - as I say I'm not set on using chainHeight :)

edit: ah, I think this is already done

Yeah that's the way it's supposed to work, alert is parsed, message is displayed until the tx actually goes live, at which point state can't be trusted and shutdown occurs. Could probably do with more testing though (I think that about everything though hehe)

Are those email addresses correct?

AFAIK (assuming mastercoin.org > omni.foundation redirection is working) - though unsure for example if @m21 still checks his Omni email account.

Re who has access to Exodus - @CraigSellars can you please advise who has the key for Exodus?