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
899 stars 377 forks source link

feat: add grc20reg that works... today #3135

Open moul opened 1 week ago

moul commented 1 week ago

Alternative to #2516
NOT(!) depending on #2743

codecov[bot] commented 1 week ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

:loudspeaker: Thoughts on this report? Let us know!

dongwon8247 commented 1 week ago

@onlyhyde @notJoon @r3v4s

moul commented 1 week ago

It seems to work, unless we consider the current behavior a bug that we want to fix. In that case, we can create gno object registries by avoiding the storage of remotely defined objects. Instead, we can store remotely defined functions that dynamically return the remote object when needed.

I’m not a big fan of this indirection because I believe Go interfaces are excellent. However, since we fully preserve Go's type safety, I think it’s a good compromise that allows for greater independence between realms.

The last checkbox in the original comment raises an open question about how we plan to manage realm deprovisionning in the future. One idea I have that would avoid the need to track which remote realms are storing a locally defined function (remote backref). We could implement a special rule in the VM so that if you:

  1. Store a remotely defined function,
  2. Deprovision the remote realm,
  3. Attempt to call the locally stored (now dead) function, instead of panicking, we could simulate that the pointer is now nil at runtime.

This would mean we only need to check if the pointer is nil to manage custom error handling, or we could allow the VM to panic due to the missing pointer.

n0izn0iz commented 1 week ago

Did you try to call mutating functions from the calling realm? The tests seem to be read-only

moul commented 1 week ago

I have some tests that perform mutations. However, if you want an advanced case, check out https://github.com/gnolang/gno/pull/2510#issuecomment-2480500066.

n0izn0iz commented 1 week ago

Can you link the mutation tests pls? I can't find them

On the atomic swap branch, the swap with registered token is not tested currently. I added the test and it doesn't work

PR: https://github.com/moul/gno/pull/21 Test result:

❯ go run ./gnovm/cmd/gno test ./examples/gno.land/r/demo/atomicswap                                                                                                                 Native
--- FAIL: TestNewGRC20Registered_Claim (0.00s)
uassert.Equal: same type but different value
    expected: 60000
    actual:   30000
uassert.Equal: same type but different value
    expected: 70000
    actual:   0
uassert.Equal: same type but different value
    expected: 60000
    actual:   30000
uassert.Equal: same type but different value
    expected: 140000
    actual:   70000
./examples/gno.land/r/demo/atomicswap: test pkg: failed: "TestNewGRC20Registered_Claim"
FAIL
FAIL    ./examples/gno.land/r/demo/atomicswap   1.17s
FAIL
FAIL
FAIL: 0 build errors, 1 test errors
exit status 1

Just FYI we have this pattern on teritori repo for a long time but mutations never worked (https://github.com/TERITORI/teritori-dapp/blob/main/gno/r/dao_registry/dao_registry.gno)

moul commented 1 week ago

Here's one: https://github.com/gnolang/gno/pull/3135/files#diff-57ef3b955ce6dd26e824d52cfa9fd0d84e2b4f67e50abdd3f5d117458a14e59cR15

You're right; the atomic swap wasn't tested, and I forgot to remove a "panic not implemented" blocker. However, the test output you shared isn't related to mutation. It's due to reusing the same "sender" and "recipient" addresses in independent tests. In other words, if you run a single unit test with -run, it works, but running everything together does not. Here is a commit that makes the sender and recipient independent for each test: 3a5acf1fd38e094346f305c37a9b54f6f1d9b4bd.

And here a21ddd5ab84687b1a4fe9d3bfb2a3afee2c2ebd5 is the commit that removes the panic and adds the missing tests for atomic swap. It works.

$>  gno test -v ./examples/gno.land/r/demo/atomicswap
=== RUN   TestNewCustomCoinSwap_Claim
--- PASS: TestNewCustomCoinSwap_Claim (0.00s)
=== RUN   TestNewCustomCoinSwap_Refund
--- PASS: TestNewCustomCoinSwap_Refund (0.00s)
=== RUN   TestNewCustomGRC20Swap_Claim
--- PASS: TestNewCustomGRC20Swap_Claim (0.00s)
=== RUN   TestNewCustomGRC20Swap_Refund
--- PASS: TestNewCustomGRC20Swap_Refund (0.00s)
=== RUN   TestNewGRC20Swap_Claim
--- PASS: TestNewGRC20Swap_Claim (0.00s)
=== RUN   TestNewGRC20Swap_Refund
--- PASS: TestNewGRC20Swap_Refund (0.00s)
=== RUN   TestRender
--- PASS: TestRender (0.00s)
ok      ./examples/gno.land/r/demo/atomicswap   1.44s
ltzmaxwell commented 4 days ago
  1. this pattern is no different with this, except for the addition of Getter for convenience, right?
    
    // PKGPATH: gno.land/r/crossrealm_test
    package crossrealm_test

import ( "std"

crossrealm "gno.land/r/demo/tests/crossrealm"

)

type fooer struct { v string }

func (f *fooer) Foo() { println("hello " + f.v + " " + std.CurrentRealm().PkgPath()) }

func (f *fooer) Mutate() { f.v = "b" }

var f *fooer

func init() { f = &fooer{v: "a"} // attach first }

func main() { crossrealm.SetFooer(f) crossrealm.CallFooerFoo()

crossrealm.MutateFooer()
crossrealm.CallFooerFoo()

}

// Output: // hello a gno.land/r/crossrealm_test // hello b gno.land/r/crossrealm_test


2. There are underlying bugs that we need to fix (though they do not block the `Getter pattern`):
```go
// PKGPATH: gno.land/r/crossrealm_test
package crossrealm_test

import (
    "std"

    crossrealm "gno.land/r/demo/tests/crossrealm"
)

type fooer struct {
    s string
}

func (f *fooer) Foo() {
    println("hello " + f.s + " " + std.CurrentRealm().PkgPath())
}

func (f *fooer) Mutate() {}

func init() {
    f := &fooer{s: "B"} // XXX, note this is incorrectly attached to "r/demo/crossrealm", by the next line.
    fg := func() crossrealm.Fooer { return f }
    crossrealm.SetFooerGetter(fg)
    crossrealm.CallFooerGetterFoo()

    f.s = "C" // XXX, so this cause a panic while modifying external realm obejct
    crossrealm.CallFooerGetterFoo()
}

func main() {
    print(".")
}

// Error:
// cannot modify external-realm or non-realm object
moul commented 3 hours ago

This pattern is no different with this, except for the addition of Getter for convenience, right?

Not exactly—it's not just for convenience but because the behavior is fundamentally different. With cross-realm objects, we encounter ownership errors due to the constraints of realm isolation. However, by using the Getter pattern to store a pointer to a function (either top-level or lambda), we avoid these ownership issues entirely. This makes the Getter pattern a unique and powerful approach that enables interactions across realms in ways that aren’t possible with direct object storage.

There are underlying bugs that we need to fix (though they do not block the Getter pattern):

👍