-
Notifications
You must be signed in to change notification settings - Fork 375
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
feat: add grc20reg that works... today #3135
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: moul <94029+moul@users.noreply.github.com>
Signed-off-by: moul <94029+moul@users.noreply.github.com>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3135 +/- ##
==========================================
- Coverage 63.79% 63.75% -0.05%
==========================================
Files 549 549
Lines 78819 79449 +630
==========================================
+ Hits 50281 50651 +370
- Misses 25146 25380 +234
- Partials 3392 3418 +26
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Signed-off-by: moul <94029+moul@users.noreply.github.com>
Signed-off-by: moul <94029+moul@users.noreply.github.com>
Signed-off-by: moul <94029+moul@users.noreply.github.com>
Signed-off-by: moul <94029+moul@users.noreply.github.com>
Signed-off-by: moul <94029+moul@users.noreply.github.com>
Signed-off-by: moul <94029+moul@users.noreply.github.com>
Signed-off-by: moul <94029+moul@users.noreply.github.com>
Signed-off-by: moul <94029+moul@users.noreply.github.com>
Signed-off-by: moul <94029+moul@users.noreply.github.com>
Signed-off-by: moul <94029+moul@users.noreply.github.com>
Signed-off-by: moul <94029+moul@users.noreply.github.com>
Signed-off-by: moul <94029+moul@users.noreply.github.com>
Signed-off-by: moul <94029+moul@users.noreply.github.com>
Signed-off-by: moul <94029+moul@users.noreply.github.com>
Signed-off-by: moul <94029+moul@users.noreply.github.com>
Signed-off-by: moul <94029+moul@users.noreply.github.com>
Signed-off-by: moul <94029+moul@users.noreply.github.com>
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:
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. |
Did you try to call mutating functions from the calling realm? |
I have some tests that perform mutations. However, if you want an advanced case, check out #2510 (comment). |
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. PR: moul#21
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) |
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 And here a21ddd5 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks valid to me 👍
type XXX func() grc20.Token
instead of agrc20.Token
directly.grc20reg
.gnovm/tests
to demonstrate the current VM's management of the cross-realm feature and support potential changes in Gno Realm and Ownership Spec #2743.atomicswap
or a similar application. (feat: add r/demo/atomicswap #2510 (comment))Token.Getter()
helper. (Works! f99654e)Alternative to #2516
NOT(!) depending on #2743