Skip to content
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

Adding notifications per issue #265 #267

Merged
merged 9 commits into from
Oct 22, 2014

Conversation

dacoinminster
Copy link
Contributor

See issue #265 for the origin of this idea

@m21
Copy link

m21 commented Oct 1, 2014

Transaction type 0 it says?
That is Simple Send.

@dacoinminster
Copy link
Contributor Author

Heh. Oops.

Silly copy/paste error corrected. I meant to put 70 in there.

Fixed. Thanks!

On Wed, Oct 1, 2014 at 12:07 PM, Michael [email protected] wrote:

Transaction type 0 it says?
That is Simple Send.


Reply to this email directly or view it on GitHub
#267 (comment).

@dexX7
Copy link
Member

dexX7 commented Oct 2, 2014

Hehe 1MpNote is nice, but this is a huge single point of failure. With P2SH up to 15 (!) signatories should be possible and I think it would be much more secure to make use of this and rather create something like an 3-of-5 (or so) "address" which also ensures consensus.

In general I'd prefer to use something like Tx 31: Publishing Data here - and (or because) this format is very limiting.

What if @achamely one day decides there is need for a notification sent out to MastercoinJ users? Or let's say, what if the company Quantum Miners Imperium likes to raise a vote and wishes to send a message to QuantumMinerToken holders? What if no longer 1MpNote is used? What if there is a different versioning scheme than x.y.z? What if there is a notification that doesn't fit into the category "holders of currency x and something with a version number" at all?

The key issue I see here is that those messages are only useful for a very specific application, scope or user group.

The immedient goal is to send update notification messages to Master Core users, but @m21 could as well just implement some method that watches all transactions that are sent from 3MaSterCoreUpDaT... without even specifying this here in the spec, because it's not consensus critical anyway.

Since providing some framework seems benefical nevertheless, I suggest to create a "send notification" transaction type instead that has a general purpose field and create another document which lists senders of notification messages and common message formats for core projects as well as guideline for users who like to use the notification feature.

Something like:

Field Type Example
Transaction version Transaction version 0
Transaction type Transaction type 71
Notification String null-terminated "U:2.3.313/2.0.0"

And somewhere else ("MasterNotifications.md" or maybe even in the mastercore repo directly?):

Master Core will read messages sent from "3MaSterCoreUpDaTeNotificaNaasdfsdf" with a format such like "U:2.3.313/2.0.0" as:

"There is a new update with version number 2.3.313 for Master Core available which is mandatory for users with pre 2.0.0 versions."

Those messages trigger an update notification. .... (just as an example)

There could of course also be "templates" for other notifications such as "notice to all quantum miners: there is a hardware defect which might result in a burning device" or "the serious-assets-company wants to raise a vote" etc. and I also think there should be some "standard" so clients know how to identify and parse those notifications.

Please also consider using type number 71 (or indeed 31) instead of 70. 70 could be used to register or define such a template, but that's probably out of the scope of this issue.

Edit: for the case of Master Core it would be very reasonable to broadcast something like a file hash sum of binaries as well, so a client can verify an update file is indeed legit.

@dacoinminster
Copy link
Contributor Author

So maybe we should modify TX31 to add an optional text field? People could then use it however they want (the numerical field could restrict version numbers affected, and the text could be the notification, for instance).

Regarding P2SH, I'm out of my league there. How does one set up a trusted source of notifications using P2SH? What tools would you use? Is it possible to create a "vanity" P2SH source like I did with vanitygen to create 1MpNote?

@m21
Copy link

m21 commented Oct 2, 2014

@dacoinminster
Copy link
Contributor Author

Doh!

Anybody object to just expanding TX31 to add a text field?

On Thu, Oct 2, 2014 at 2:16 PM, Michael [email protected] wrote:

JR TX70 is taken already:

  • * 70: Change Property Issuer on Record
    <#148d2b87d0c97796_change-issuer-on-record-for-a-property>

https://github.com/mastercoin-MSC/spec/pull/257/files


Reply to this email directly or view it on GitHub
#267 (comment).

@dexX7
Copy link
Member

dexX7 commented Oct 2, 2014

D'oh, and I wanted the 7 for games. ;)

Jokes aside: @dacoinminster: maybe we could just use:

  1. Transaction Version: 0
  2. Transaction Type: 31
  3. General Purpose (null-string?): Notification

Or as you proposed:

  1. Transaction Version: 0
  2. Transaction Type: 31
  3. Ecosystem: x
  4. General Purpose (null-string?): Notification

Or maybe even:

  1. Transaction Version: 0
  2. Transaction Type: 31
  3. Ecosystem: x
  4. Data: xxx
  5. Notification

But neither "data" (as number) nor "ecosystem" are really fitting for this purpose. What do you think about a slim version such with simply a notification field and the current prototype of tx 31 turns into tx 32?

Side note: we should be careful that we don't call the field "message" field or something similar.

Re: multisig and P2SH -- I'll post something about this in the following days or maybe better setup some demo. This isn't all that difficult, but rather difficult to explain for me at the moment. ;) The result is very similar to m-of-n multisig, but not limited to 3 signatories.

Low level intro: https://bitcoin.org/en/developer-guide#p2sh-scripts + the killer app of P2SH multisig (but unfortunally not usable for this purpose, unless we fork the whole project, I think..): https://copay.io/

@wanderingbort
Copy link
Contributor

I understand the concerns of using the bitcoin alert system and relaying. However, pushing alert data direct to the blockchain seems like a bad idea. We may use it sparingly, but others may want to send out hourly reports to their users and those would be immortalized in the gargantuan block chain. Mining a transaction into a block is forever.

Understandably we operate a second level protocol that requires us to push data into the blockchain to operate. We should take some responsibility to (ab)use this privilege in a responsible way. The block chain seems too permanent for transient notification data.

As an alternative, I would propose an official "side channel" (RSS perhaps, HTTP, gopher) and then a blockchain recorded transaction that announces the address / verification that can be used to find and validate that you are receiving the official stream. This announcement would only have to be published when those values change and the rest of the time our clients can simple read from the "official" stream.

Each property could opt to list its official stream too and then it becomes pub/sub system with the data delivery achieved by something more appropriate (preferably with widely known and accepted standards / servers / clients).

The blockchain is a good tool but we really should not treat it like our only option.

@dexX7
Copy link
Member

dexX7 commented Oct 3, 2014

No, of course not, I really don't want a message system and #250 is all about off-chain storage, but in this case relying solely on an "unreliable" data source is not sufficient.

As an alternative, I would propose an official "side channel" (RSS perhaps, HTTP, gopher) and then a blockchain recorded transaction that announces the address / verification that can be used to find and validate that you are receiving the official stream. This announcement would only have to be published when those values change and the rest of the time our clients can simple read from the "official" stream.

This sounds reasonable and like the way to go. "There is a new version and here is the proof this version is legit" - this is what you have in mind, right? Would be perfectly fine. It would also be sufficient, if the Master Core team pushes a file hash via a well known entity on the chain and MC is able to identify those. And if the actual update isn't available due to some unreliable external source or whatever, then that's fine too, because the user/client is at least aware of a potentially very different view of world.

What concerns me is how fragile Mastercoin is due to it's global state and so far all other efforts to tackle the underlying problem of fragility were fruitless.

Sending a client update notification message directly over the chain ensures it's available when needed the most. - And it could be done tomorrow.

@m21
Copy link

m21 commented Oct 3, 2014

Mainly as a response to @wanderingbort I do not see any good way to "alert" users of Master Protocol outside of our normal MP-style transactions.
P2P or outside channels are not reliable or do not provide "push" style notifications.
And anybody anywhere has always been allowed to dump data into the blockchain, unrelated to MP or anything we do.

So for the official Foundation "alerts" of which we don't expect there to be many this method is pretty much harmless.
I am not sure I like JR extending it to every token issuer though. They should be using outside channels.
So can we be just like Bitcoin - build the listening mechanism into our clients, for Foundation's alerts and not the broadcasting mechanism.

@dexX7
Copy link
Member

dexX7 commented Oct 3, 2014

I am not sure I like JR extending it to every token issuer though.

Keep one thing in mind: it's not consensus critical. If you don't like it, ignore it and don't implement it. (and my personal opinion: unless there is some game changing event I would not go further than support for a notification for critical projects). But likewise another user could setup a service that parses and fetches every junk message one could imagine. This is why I also mentioned, I'm not sure, if the "spec" is the right place for this.

But I see some benefit, if there were a common format and standard. A situation with many messages and spam without intercompability that is nevertheless out there is worse than one where one could opt-in due to a similar schema that is used.

@m21
Copy link

m21 commented Oct 3, 2014

For critical alerts from the Foundation the spec is the right place for it I think.
We implement it that way in the clients (per spec). Its use is very similar to Bitcoin's alert except it's delivered via the blockchain and not via P2P which we can't use.

I am kinda against anyone else using it as a general purpose announcement mechanism (token issuers) and polluting the blockchain using our software.

@dacoinminster
Copy link
Contributor Author

I got an interesting comment from Zathras via email:

On Fri, Oct 3, 2014 at 6:51 AM, Zathras Crypto [email protected] wrote:
Hey J.R.,

I had in mind an array of trusted addresses rather than singular.

I note DexX's comments re P2SH but even with multisig we make this a singular key concept. Since the solution would be under our control, we can array a number of approved addresses and remove/add to these as developers leave/join the project. For example let's say you used 1ZathrasXXXX in the array that key could be revoked from the source if there was any change to my affiliation with the project without needing to rekey and redistribute privkeys to the remaining devs.

Still catching up, making my way back to Aus and a little jetlagged at the mo, so might not be making too much sense :)

Thanks
Z

Thanks Zathras,

That's an interesting idea - multiple authorized senders makes sense. Too many hurdles could backfire (i.e. you need to send an urgent notification, but can't reach enough signatories in a timely manner). I'm fine with giving the private key to a few trusted people OR having multiple addresses authorized to send.

@dexX7
Copy link
Member

dexX7 commented Oct 5, 2014

@zathras-crypto: ... an address array?
Like "let's define a bunch of different addresses in mastercore"? Sounds good.

Master Core is the client reading those messages, so it should be easy to change the values when needed anyway - maybe even on the fly? For example via a startup argument like mastercored --add-update-channel 1ZathrasXXXXxxxXx.

Sending and creating P2SH transactions with one client that holds all keys is easy, but I noticed during the testing it might not be easy to sign a mutlisig transaction in a more "distributed" way, e.g.:"michael starts to sign the tx, faiz continues, zathras goes on...", because the internal wallet controller of core is used and there is no exposure of the raw unsigned transaction at all at the moment..

It's easy, if all private keys are used by the client which also creates the transaction and signs the transaction, but that's not really a win.

Anyway, here is the log of the whole process, starting with the creation of a P2SH wallet and sending a few TMSC back and forth on testnet:

https://gist.github.com/dexX7/96c7ca99f909c15fad68


Details aside.

Is this final? Or do you prefer one of the other combinations?

Field Type Example
Transaction version Transaction version 0
Transaction type Transaction type 31
Notification String null-terminated "U:2.3.313/2.0.0"

I don't like a fixed field type for the notification field, but in the end that's also up to the client.

@dacoinminster
Copy link
Contributor Author

I think I will update my pull request to be this. I will add notes to TX31 (and probably change the title) to talk about how this can be used for notifications.

@dacoinminster
Copy link
Contributor Author

I have completely reworked this pull request to use TX31 instead. I like it much better this way. Please take a look and/or merge!

Thanks!

@dexX7
Copy link
Member

dexX7 commented Oct 8, 2014

How would you use ecosystem and the data (number) field in this case?

@zathras-crypto
Copy link
Contributor

Hey guys,

For clarity what I had in mind was purely an alerting system, notifications in the blockchain I think needs far more discussion and I'm not a fan myself. Extending this to any issuer is really going to get us some accusations of supporting spam.

I initially envisioned that as we may discover critical bugs as we're still in alpha stages, especially when we'll be doing a UI Windows alpha release soon and wanted to make sure we had a way to reach a likely increase in users testing.

TX31 I would perhaps argue against. At least as long as we have Class B. Class C (see here very rough initial draft http://pastie.org/private/lqtwnyyt8undpgth1tlw) would move us to prunable outputs and then perhaps we could enable TX31 with some caveats:

  • Only supported with Class C (I'm no expert on the political landscape - I leave that to smarter folk than I - but if (and I say if) we're going to allow messages that do not actually perform an MP state related action, let's do so as cleanly as possible)
  • Only allow enough bytes to fit in an OP_RETURN output - yes this is very limiting but still enough for a version message, or short alert, URL to a blog post or announcement or something etc. Clients should not automatically visit/download further info from any URL's (ever), simply provide to the user the option to visit (otherwise that's enabling someone to setup a webserver to gather a list of MP node IPs).

And suggested changes:

  • Add a propertyID field so issuers can nominate the property they are notifying on
  • Remove ecosystem (easily derived from propertyID field)
  • Remove data (easily merged into memo/notification field)

Overall with the changes that would support a message/notification size of somewhere roughly 30 bytes (depending on how we implement Class C). Again sounds restrictive, and it is, I provide the following limited examples:

<VER>12345678</VER>
This is a note for you to read
HIGH VOLTAGE DO NOT CLIMB
domainname.com/blog.php?p=244
mylongdomainname.com/shorturl
http://tinyurl.com/abcdefgh

As usual just my 2 cents :) And please don't read this as "Zathras supports adding notifications to Master Protocol" because I do not, but good debate comes from looking at all the angles, so I wanted to provide a perhaps middle ground alternative too.

Thanks
Z

@dexX7
Copy link
Member

dexX7 commented Oct 9, 2014

For clarity what I had in mind was purely an alerting system, notifications in the blockchain I think needs far more discussion and I'm not a fan myself. Extending this to any issuer is really going to get us some accusations of supporting spam.

Any notification is most likely bound to a very specific context and application, so it's imho a question of: is there reason to provide a minimal template (v: 0, tx: 31, data: xxx, ...) or should this be left out of the spec completely and rather mentioned in the mastercore docs?

Again sounds restrictive, and it is, I provide the following limited examples: ...

Well, what do you intend? Provide an option in the client to "subscribe" to "message feeds"? Or was this just an example? If it's not "officially" supported as a whole, then there is no need to define limits or whatsoever in the first place, because adding junk to transactions is no protocol violation and thus ... what might be junk for me, could as well be a bunch of messages for someone else. ;)

@dacoinminster
Copy link
Contributor Author

To be clear, I do NOT support our UI displaying notifications from anybody
to anybody. Our UI would only display our own notifications, NOT from
issuer X to users of a property. The question is: where to put OUR
notifications. Adding text field to TX31 seems low risk.

On Wed, Oct 8, 2014 at 9:59 PM, dexX7 [email protected] wrote:

For clarity what I had in mind was purely an alerting system,
notifications in the blockchain I think needs far more discussion and I'm
not a fan myself. Extending this to any issuer is really going to get us
some accusations of supporting spam.

Any notification is most likely bound to a very specific context and
application, so it's imho a question of: is there reason to provide a
minimal template (v: 0, tx: 31, data: xxx, ...) or should this be left out
of the spec completely and rather mentioned in the mastercore docs?

Again sounds restrictive, and it is, I provide the following limited
examples: ...

Well, what do you intend? Provide an option in the client to "subscribe"
to "message feeds"? Or was this just an example? If it's not "officially"
supported as a whole, then there is no need to define limits or whatsoever
in the first place, because adding junk to transactions is no protocol
violation and thus ... what might be junk for me, could as well be a bunch
of messages for someone else. ;)


Reply to this email directly or view it on GitHub
#267 (comment).

@zathras-crypto
Copy link
Contributor

To be clear, I do NOT support our UI displaying notifications from anybody to anybody.

I did not say anybody, perhaps I should have been more explicit but I meant notifications from issuers of properties to holders of said properties. To be fair it was added to the spec under a transaction specifically intended for anybody to publish data (a tx called 'Publish data') so thinking there would be other users than us devs isn't too much of a stretch :p

@zathras-crypto
Copy link
Contributor

Reflecting on this a little, perhaps I kind of agree with DexX - if we (the foundation) are the only consumers why have it in the spec at all? We could just devise a custom message type that MasterCore recognizes and uses to notify users.

@dacoinminster
Copy link
Contributor Author

I'm cool with that. Where should it be specified, if not in the spec?

On Fri, Oct 10, 2014 at 11:35 PM, zathras-crypto [email protected]
wrote:

Reflecting on this a little, perhaps I kind of agree with DexX - if we
(the foundation) are the only consumers why have it in the spec at all? We
could just devise a custom message type that MasterCore recognizes and uses
to notify users.


Reply to this email directly or view it on GitHub
#267 (comment).

@dexX7
Copy link
Member

dexX7 commented Oct 13, 2014

In the documentation of the application - in this case Master Core. I sort of would enjoy a general purpose notification transaction type, but on the other hand: it wouldn't hurt if it's not specified and it's always possible to add such a transaction later - if there is need for.

@dacoinminster
Copy link
Contributor Author

In that case, I'll get rid of the pull request, MasterCore devs can publish
and display the notifications however they like. Any objections?

On Mon, Oct 13, 2014 at 10:15 AM, dexX7 [email protected] wrote:

In the documentation of the application - in this case Master Core. I sort
of would enjoy a general purpose notification transaction type, but on the
other hand: it wouldn't hurt if it's not specified and it's always possible
to add such a transaction later - if there is need for.


Reply to this email directly or view it on GitHub
#267 (comment).

@marv-engine
Copy link

@zathras-crypto

... I meant notifications from issuers of properties to holders of said properties

We have to be aware of the possibility for spam. If/when there's a version of Send to Owners that lets a user send token X to all holders of token Y, the issuer (likely that user) would now be able to spam everyone who holds token Y.

If users can choose to block unsolicited/unaccepted incoming sends (or at least STOs), they would be able to protect themselves from getting spammed with unwanted tokens and notifications.

@dexX7
Copy link
Member

dexX7 commented Oct 13, 2014

Token and potential message spam is solely an UI issue imho.


When I first read about the broadcast transaction (at the very beginning) I wasn't thinking about bets, but message streams that users could opt-in, sort of like a RSS feed or Twitter.

Over time I realized the sooner any non-critical data is moved off-chain, the better. This is why I'm still split on this one and I'm certain spec updates (and to a greater degree Master Core updates) can be considered as critical, but anything else probably not.

On the other hand: providing a notification/message transaction equals in practice simply providing a way to encode arbitrary data - and that's neutral and up to each user who (ab-) uses such a mechanism. If we get some day to the point where MSC transactions are more efficient, it may become a handy tool that could be build upon - example use case: a proof-of-existence clone.

@dacoinminster
Copy link
Contributor Author

Notifications are now out of scope of the spec, so I reverted all my commits, so pull request is now a no-op. There might be a cleaner way to do this, but git confuses me sometimes . . . .

dacoinminster pushed a commit that referenced this pull request Oct 22, 2014
Pull request of commits and reverts of those commits to get my copy synced up with Master. There's probably a better way . . . 

(THIS MERGE HAS NO REAL CHANGES)
@dacoinminster dacoinminster merged commit dbfbae3 into OmniLayer:master Oct 22, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants