Neo: Notification Counter/index (neo 3)

[igormcoelho]

I think an useful feature for Neo 3 is the ability to mark a notification. One example is when a contract wants to make sure some event happened (example "Transfer" 10 NEO), and mark it as "consumed" or "visited". A boolean would be enough, and when marked, this mark wouldnt go away. In a next step on invocarion stack,another contract (or the same) could verify that thos notificarion has been consumed already (and ignore it).
This is specially helpful when more than one contract operation is performed on same transaction.

[igormcoelho]

Any.thoughts? @shargon? @jsolman ? @vncoelho?

[vncoelho]

I believe it is a good idea and direction, but how to ensure that the SC will consume it?

[shargon]

I think that this is a good idea

[igormcoelho]

First we need to.implement https://github.com/neo-project/neo/issues/308
Then.we add an interop System.Notification.Hide, that hides current notification (marks Hidden attribute as true)
After this, we create another interop System.Notification.GetAttributes (one of which is "hidden" flag). An alternative will be a SetAttributes, instead of Hide interop,but this is unsafe as we could "unhide" notifications.

[lock9]

I've been visiting this for some time and the only thing I can say is that I don't like this design.
I kind of understand the issue, I just don't think I like the solution 😅

The developers will never understand this... at least not with the "hide" keyword, it looks there is something wrong here 🤔

Can't we make this with a different design? Have some sort of "shareable context" that is used to exchange information during dynamic invokes or other smart contract calls? In Android, when we start another activity, we put everything they need in a bag and start the activity (with the bag).
The activity checks the bag to see what has happened before to build the screen properly. For example: you may or may not want to show the back button depending where the user came from / did before.

Instead of Context.Put, something like CurrentExecutionContext.Put.

We can add the scripthash as prefix to ensure that each smart contract can only edit their own storage, but can read all prefixes.

[uvmetal]

Why not just mark it read and let the notification list filter on the various states?

I agree "hide" or "hidden" is cryptic. It's a notification, I vote we keep the language relevant.

[igormcoelho]

Ok, he it goes a completely alternative design @lock9 . If each notification as a unique hash (we have to discuss how to generate that), you could save it on contract storage (after "processing" the notification), but contract loop would be responsible for managing this whole situation (what has been processed, and what hasn't). Without a unique hash (like now), it's even more complicated, but it's also doable. Imagine someone sends 5 NEO twice, you would have two notifications "transfer 5 NEO from me to you". You could process one by one, discover it's 10 NEO afterall, store string on map and count it... it's doable, perhaps a raw solution easier for developers to understand, but looks like stone age to me.

When you have a notification on your phone, and you "see it", what happens? You "dismiss" it? You "hide" it? You "consume" it? You "process" it? You "read" it?
This was the inspiration for the idea, but wording can be improved, now that's crystal clear ;)

@uvmetal I like the word "read", let's try this approach.

[shargon]

Consume notification is better

[igormcoelho]

The problem is see with "consume", is that it should probably disappear after "consumed". I didn't want that... no sure why, but I think there may be situations where removing/consuming certain notifications, without the awareness of the contract, may change logic. Imagine I notify A, then B, then C. Some random contract in between "consumes" B. Then, A + C gives a different logic than A,B,C... it could happen.
But if you mean "consume" just hiding it, and people still getting able to see that somehow, it works for me. In fact, we have to decide two things:
1) is this feature really useful and necessary?
2) how to properly name it and define it?

[shargon]

GetAllNotifications, GetUnconsumedNotifications , GetConsumedNotifications and CosumeNotification
This cover all use cases

[lock9]

@shargon @igormcoelho what prevents us to create ephemeral storage to use with generic data?
Today we are using for notifications, but couldn't we have something more generic? I know where this is going, and I think that although we are considering notifications, it is possible to support much more custom logic.

[shargon]

if we can receive notifications is the same and are associated to the script hash

[igormcoelho]

Shargon... perhaps we should think a bit more on filtering over an Enumerator interop stack item. This applies to many other cases... so we put options on every notification (consumed = true).
So, in this case, we only need GetAllNotifications, and it gives us an Enumerator.
The next thing is: how to syntatically write:
cs Enumerator e = GetEnumerator(); // GetAllNotifications(); Enumerator e2 = e.Filter(x => x.IsConsumed()); // sorry for my poor C#
How could Filter appear in neovm context? @shargon @lightszero @erikzhang @lock9
I only imagine an OPEVAL thing... but the must be something simpler... perhaps we pass a delegated function as parameter? One that returns bool for the expected parameters?

[lock9]

@igormcoelho a delegate is a good way to solve this. Not that I can see all steps, but anything that is not OPEVAL is good to me 😂
Instead of having all these "consuming" mechanisms, why can't we just use custom logic in the filtering method?
Isn't "consuming" a notification, the same as creating a new notification saying "notificationConsumed", "scripthash"?

If we can get all notifications, and filter it based on it's content, we don't need all these consuming stuff, do we?
Does this makes sense?

[igormcoelho]

Isn't "consuming" a notification, the same as creating a new notification saying "notificationConsumed", "scripthash"?

It's a great idea, but still, we don't yet have an unique field in a Notification... "scripthash" is the caller scripthash, but it may issue two identical notifications, and we would need to perhaps just evaluate one of them. If we put forward some unique fields for notifications, we can manage using local storage, or another notification like you said... but since we don't have that, perhaps it's just easier to mark it as "read", or is it too crazy?

[lock9]

I think that the best path is generic ephemeral storage, and like Shargon mentioned, the notifications can be used for that, however, I think we need this storage, to be able to pass "logic" between app calls. We don't want to call that a "notification".
So
1 - A ephemeral storage looks a good way to solve this
2 - Notifications can be used for the same purpose, but I think that "notifications" are used for "notifying", and this storage we need here is used for "local logic" only.

Edit: What about this #898 ? Maybe the name of this issue is not 'adequate'? Or is it the same thing as #898 ? 🤔

[igormcoelho]

898 is an implementation of a necessary proposal by Erik https://github.com/neo-project/neo/issues/308

Even without changing notification state (like proposed here), we still need to be able to get current notifications, which is the point of that PR. After that, we can see how to manage this situation.

[lock9]

@igormcoelho but what is the scope of this issue then? I'm sorry, I didn't understand the difference yet.
The title is "Read notifications", the #308 and #898 are "GetNotifications". What is the difference? 🤔

[igormcoelho]

GetNotifications just gets the full list, or a list filtered by scripthash issuer (implemented already by shargon).
This one proposes "read"/"consumes"/"hides" which affects an individual Notification (after some GetNotifications) and allows this change to be remembered in the future. It changes the State of a specific Notification (state += read).

[lock9]

@igormcoelho I think that the best solution for this is the second storage, why are we limiting this "storage' to handle notifications only? I think that notifications are used to notify. We need ephemeral storage to help us with logic, this is different.

[igormcoelho]

I agree with you, storage is better, but we didnt have a unique identifier... until now :)

The simplest solution was right before our eyes, lets add a Notification counter on notifications, so we can flag them by #id. Regarding storage, we can allow a GetStorageContext(0x00000..000) to hold temporary information,never persisted, or we can simply use global variables for that,much simpler.

Thanks Ricardo for making me see the obvious solution ;)

[lock9]

In Android, we use "PutExtra" and "GetExtra" to use this other "storage". It is not actually a storage in the disk, it is just used to pass information from one activity to another.
It solves a ton of problems, I think this can be a good solution for us.

[igormcoelho]

@lock9 for ephemeral storage, we could have a TemporaryStorageContext (different from a Contract Context). I think this is useful for many applications, similar to a Global variable, but with access in interleaved contract executions:

• Contract A (writes on TmpStrgContext)
  
  
  
  • invokes Contract B
    
    
    
    
    
    • invokes Contract A (still has data on TmpStrgContext)
    
    
    
  
  

Question is: should this data be shared among different contracts? does it makes sense? I think that Tmp tied to a single contract is enough for the situations we describe here.
If other situations are necessary, we could have some shared InvocationStorageContext (for inter-contract communications).