mastercoin-MSC / mastercore

mastercore info
mastercoin.org
MIT License
24 stars 11 forks source link

Add alerting and getinfo_MP #207

Closed zathras-crypto closed 9 years ago

zathras-crypto commented 9 years ago

Please see notes here guys http://pastie.org/pastes/9733526/text?key=luz9fdv1whtvfhn1sjvckg

Before merging can we grab an hour to build this and run over how it works/test it out.

Thanks Z

dexX7 commented 9 years ago

What I like:

I left some notes in-line and unfortunally I'm unable to test the system for some reason I'm not yet aware of. I updated the code accordingly to respect an address of mine. Please see:

https://gist.githubusercontent.com/dexX7/b1f6ec90b2d1c4485e1b/raw/

For some reason the first two bytes were parsed as zero. I'm also unable to find any other alert entries in the mastercore.log (which I assume should exist, if you tested this).

Looks like some flaw in the log system, because when decoding the transaction manually, I'm able to extract:

ffffffff313a3330393630303a303a5468697320696e666f20616c657274206973206e6f742076657273696f6e20746172676574656420616e64206578706972657320617420626c6f636b20686569676874203330393630302e000000000000000000000000000000000000000000000000000000000000

getinfo_MP works, but without showing an alert, but I'm sure, I started the correct build.. ;)

I'm in favor of merging this either way, because it's an improvement and needed. But let me raise a few points nevertheless and one point is quite relevant in particular:

though of course difficult to test

I'll code some regtests, once I get this running.

This message is displayed in the UI and getinfo_MP RPC call which should be polled by integrators every n minutes.

Requiring integrators to query getinfo_MP and parse it's result is extremely flawed in my opinion. This implies serious steps and work needed for something that seems to be rather easy to accomplish with the basis you already build: every block transactions are parsed, so the alert is noticed implicitly already.

I suggest to drop the approach of using isTransactionTypeAllowed and think it would be more straight forward to force MC into safemode, if it's not safe to operate. This was the early behavior of the alert system and it's still the case, if a large fork is noticed.

The related code: https://github.com/mastercoin-MSC/mastercore/blob/mscore-0.0.9/src/main.cpp#L3204-L3254 https://github.com/mastercoin-MSC/mastercore/blob/mscore-0.0.9/src/rpcserver.cpp#L880-L883

I'll post another post probably soon, but right now I suggest to add something like:

if (fMasterCoreAlert)
    {
        nPriority = 500;
        strStatusBar = strRPC = _("Warning: put an alert message here.");
    }

... into string GetWarnings(string strFor) of main.cpp.

strRPC is checked by every RPC call and if the call is not explicitly flagged as "OK for safemode", then it's cancelled. If strStatusBar is not empty, the yellow UI notice with the text specified is set, so it's also not required to manually manipulate the UI. :)