gnolang / gno

Gno: An interpreted, stack-based Go virtual machine to build succinct and composable apps + Gno.land: a blockchain for timeless code and fair open-source
https://gno.land/
Other
849 stars 345 forks source link

emit & event built-in functions #575

Closed r3v4s closed 2 months ago

r3v4s commented 1 year ago

I opened this issue to discuss about emit (i.e event subscribe).

In Solidity, you can generate logs using emit & event methods

Theses concepts are used to communicate between Ethereum and the outside world.

Questions

Would there be any similar ways to get Gno's event from the outside world?

Suggestions

Pub-sub

  1. We can use the ws subscribe concept from Tendermint, which has been widely used.
  2. We can use a new method that the Tx indexer will be using (I think #546, being developed by @zivkovicmilos, has some ideas that we can bring to this issue).
    • in #546 picture, some indexer will output to ws. If it uses ws with extra method for subscribe, I think we can bring this concept to emit.
moul commented 1 year ago

Let's explore implementing this feature. Some initial questions and suggestions:

Please share your thoughts.

moul commented 1 year ago

I have a new idea: what if we use the following code snippet instead?

import "log"

func Transfer(amount grc20.Token) {
    // ...
    log.Print("foobar")
}

This would append a transaction receipt similar to the following:

{"caller":"gno.land/r/demo/foo20","msg":"foobar"}

What do you think?

r3v4s commented 1 year ago

Let's explore implementing this feature. Some initial questions and suggestions:

  • Should this feature be restricted to those replaying transactions, or should we make the data available to other contracts ([WIP]feat: Protocol to support contract-contract interaction #473)?

  • for d-app developers, making data available to other contracts will be more flexible(or you know.. eaiser to do something)

  • Should we use std.Emit or std.Log for naming convention?

  • if not only emit but event is going to be implement, I think std.Emit is good, but without event std.Log looks more intuitive

  • Should we use std.Log(interface{}) or std.Log("key1", "value1", "key2", "value2") for logging?

  • shouldn't it be std.Log(args ...interface{}) ?? we don't know how many arguments will be passed

r3v4s commented 1 year ago

I have a new idea: what if we use the following code snippet instead?

import "log"

func Transfer(amount grc20.Token) {
    // ...
    log.Print("foobar")
}

This would append a transaction receipt similar to the following:

{"caller":"gno.land/r/demo/foo20","msg":"foobar"}

What do you think?

this looks very straightforward + easy to understand + very simple.

But to be sure is that log package will be from go's log? If it is, I can't think any better ways, but if not it might give developers confuse

moul commented 1 year ago

Yes, my suggestion is to port and adapt the official log package while maintaining API compatibility as closely as possible.

This will allow us to have transaction receipts when on-chain, while also retaining the standard stderr logging feature when using gnodev.

ltzmaxwell commented 1 year ago

But to be sure is that log package will be from go's log? If it is, I can't think any better ways, but if not it might give developers confuse

The name of log does confuse a bit, I will expect log.print() print out something in contract only, without any other side effect.

thehowl commented 1 year ago

I personally think that the name "log" should not be used for communicating and using events; in Go it is used to add a "human interface" and what is logged should not be parsed or read by other machines - aside from what @ltzmaxwell pointed out as well.

I'm more in favour of something in std or similar, although honestly I think in Gno we could just use the language and make pub/sub not a stdlib feature but a realm, as I personally think that pub/sub resides more in the realm of "useful" rather than "founding feature" - aside from the fact that having it as a realm would serve as a further demonstration of the power of stateful realms.

Here's a way I could imagine this being used:

package foo // import "gno.land/r/demo/foo"

import "gno.land/r/demo/broker"

type X struct {
    A int
}

const (
    // EventFoo is emitted when ...
    // Type: [X]
    EventFoo = "foo"
)

func Emit() {
    // As a guideline (and possibly future linter), a package should always emit the same
    // type, and it should be documented like in this case.
    broker.Emit(EventFoo, X{1})
}
package bar // import "gno.land/r/demo/bar"

import (
    "gno.land/r/demo/foo"
    "gno.land/r/demo/broker"
)

func init() {
    // There could be runtime errors (ie. Handler's argument type is not that emitted by pkg foo)
    // so this is an idea of how to handle them.
    broker.OnError(errorHandler)
    // The listeners are scoped to the calling realm.
    // Other APIs could be SetListener(pkg, evt string, fn interface{}), RemoveListeners(pkg, evt string)
    broker.AddListener("gno.land/r/demo/foo", foo.EventFoo, Handler)
}

func Handler(x foo.X) {
   _ = x.A
}
r3v4s commented 1 year ago

Since emit is actually action of Log in EVM, I thought reusing go's log does fit into this circumstance. But as @ltzmaxwell said some devs won't expect log.xx does something else rather than what it does in go.

If we want this has to be LOG we can just make new method for gno only(idk... maybe Log.PrintState), or if it doesn't have to be LOG, personally I prefer std.* stuff like @thehowl

BTW @thehowl, making it realm is very interesting stuff, but I'm worried about situation where we have to upgrade pub/sub contracts. What do you think??

thehowl commented 1 year ago

@r3v4s Are you referring to updating the broker package from my examples, or its users? Could you specify what your concern is? :)

r3v4s commented 1 year ago

@r3v4s Are you referring to updating the broker package from my examples, or its users? Could you specify what your concern is? :)

updating broker package from your examples. AFAIK, we can't modify contract without change package path(or address).

Lets say...

  1. we published gno.land/r/broker
  2. other user(such as d-app devs) import gno.land/r/broker into their realm
  3. guess what, we found there is a bug(or critical security issue) to gno.land/r/broker
  4. we made patch and deployed gno.land/r/broker_v2
  5. maybe we can pause gno.land/r/broker but it will make other realm(from step 2) crash

I had similar experience in EVM, so this is my concern.

Speaking of which, maybe we can make proxy contract for gno.land/r/broker to avoid above situation :D

thehowl commented 1 year ago

@r3v4s I spent a few words explaining how I think we should handle contract upgrades while retaining state information in this comment - so it should allow us to change broker while still keeping the data :)

anarcher commented 1 year ago
import "std"

  e := std.NewEvent("admin_added") // MakeEvent? // Native go wrapper  // Event Prefix: `gnovm-`
  e.AddAttributes("addr",newAdminAddr) // key string, value string 

IMHO, I'm not sure, but what if we treated the event that occurs in the contract as one of abci.Event?

jaekwon commented 1 year ago

I see two things.

std.Emit(std.Event)

log.Log(fmt, args...)

Rather than Tendermint1's pubsub which is made to be asynchronous (unnecessarily I would add), I would go with tm2's synchronous simple EventSwitch, which with little modification should be portable go gno. So this gno EventSwitch should be used for gno in-logic events, neither std.Emit nor log.Log. The user, if needed, can easily write a function that calls both std.Emit (or log.Log) AND EventSwitch.FireEvent.

"log" to me is something cheap, and something that doesn't have any side effects. While std.Emit(std.Event) could potentially have side effects, as in, become an input to subsequent logic (in the current or a later transaction), I think there's value to having a merkle-root included event that is guaranteed to be cheap and (direct) side-effect free, where the intent is to produce a byte sequence that can be parsed from clients like wallets. If you want to emit an event that is to be read by later logic, where the event includes a (potentially) mutable object, then tm2.EventSwitch can handle that better, or some custom pubsub system that uses the persistence of realms, as @thehowl mentioned. In other words, some function probably should exist that does nothing but produce amino bytes that get included in the merkle root, and nothing else; and std.Emit should be that.

Must log.Log generate abci.Events even when the tx fails? That would be very useful, but there's as of yet no way to do this. It wouldn't be hard to implement -- just need to define via PackageNode.DefineNative, to stash the logs somewhere. This function should be defined in gnoland (not gnovm), since it is a particular quirk of the usage of gno, and the gno VM doesn't need to be aware of it. BTW tm2/pkgs/sdk/vm should be moved into gno.land/pkgs/sdk/vm, then afterwards this log.Log maybe can be injected by modifying what is currently tm2/pkgs/sdk/vm/builts.go. // @piux2 @moul I suggest we move it to gno.land/pkgs/sdk/vm sooner than later.

anarcher commented 1 year ago

I'm going (would like) to start a PR about std.EmitEvent() using abci.Event :-) I probably won't be able to include it in this PR, but I think gas costs for using event should be added later.

moul commented 3 months ago

The initial implementation could be:

  1. std.Emit can store events occurring in the transaction in gnovm/vmkeeper, aggregate, format them, and return in abci.Event with the result.
  2. A websocket pubsub system can be set up to receive notifications for new events that match a filter.
zivkovicmilos commented 2 months ago

Closed as part of #1653 🎉