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

[gnovm/avl] Removing keys from AVL trees does not work #2266

Open
leohhhn opened this issue Jun 3, 2024 · 6 comments · May be fixed by #2992
Open

[gnovm/avl] Removing keys from AVL trees does not work #2266

leohhhn opened this issue Jun 3, 2024 · 6 comments · May be fixed by #2992
Assignees
Labels
🐞 bug Something isn't working 📦 🤖 gnovm Issues or PRs gnovm related

Comments

@leohhhn
Copy link
Contributor

leohhhn commented Jun 3, 2024

Description

This is a bit of a weird issue @ajnavarro and I came across while fixing the blog package. Namely, we implemented this function, which removes a pointer to a post from three trees. It makes sure to get the correct keys before removing anything from the trees, to avoid any possible bugs. However, when calling this function, you get the following errror:

gnokey maketx call -pkgpath "gno.land/r/gnoland/blog" -func "ModRemovePost" -gas-fee 1000000ugnot -gas-wanted 2000000 -send "" -broadcast -chainid "dev" -args "peace" -remote "tcp://127.0.0.1:26657" main
Enter password.

--= Error =--
Data: deleted descendants should not have a reference count of less than zero
Msg Traces:
    0  /Users/sasurai/Desktop/gno/gno/tm2/pkg/crypto/keys/client/maketx.go:213 - deliver transaction failed: log:msg:0,success:false,log:--= Error =--
Data: &errors.errorString{s:"deleted descendants should not have a reference count of less than zero"}
Msg Traces:
    0  /Users/sasurai/Desktop/gno/gno/gno.land/pkg/sdk/vm/keeper.go:318 - VM call panic: deleted descendants should not have a reference count of less than zero
Machine:
    CheckTypes: false
    Op: [OpHalt]
    Values: (len: 1)
          #0 (ModRemovePost func(slug string)())
    Exprs:
    Stmts:
    Blocks:
          @(51) Block(ID:0000000000000000000000000000000000000000:0,Addr:0x14004bd0f00,Source:func ModRemovePost(slug (const-t...,Parent:0x14004bd01e0)
            slug: ("peace" string)
 (s vals) @(51) Block(ID:0000000000000000000000000000000000000000:0,Addr:0x140267dbc28,Source:func ModRemovePost(slug (const-t...,Parent:0x140262b9228)
            slug: ( string)
 (s typs) @(51) [string]
          @(182) Block(ID:b267d23c27c167591da35b2b4bd51435eddcd5eb:3,Addr:0x14004bd01e0,Source:ref(gno.land/r/gnoland/blog/admi...,Parent:0x14004bd0000)
            (RefNode names not shown)
 (s vals) @(182) Block(ID:0000000000000000000000000000000000000000:0,Addr:0x140262b9228,Source:file{ package gnoblog; import st...,Parent:0x140262b9b28)
            std: (package(std std) package{})
            strings: (package(strings strings) package{})
            avl: (package(avl gno.land/p/demo/avl) package{})
 (s typs) @(182) [package{} package{} package{}]
          @(368) gno.land/r/gnoland/blog
    Blocks (other):
    Frames:
          #0 [FRAME FUNC:ModRemovePost RECV:(undefined) (1 args) 1/0/0/0/1 LASTPKG:main LASTRLM:Realm(nil)]
    Realm:
      gno.land/r/gnoland/blog
    Exceptions:


Stack Trace:
    0  /Users/sasurai/Desktop/gno/gno/tm2/pkg/errors/errors.go:20
    1  /Users/sasurai/Desktop/gno/gno/gno.land/pkg/sdk/vm/keeper.go:318
    2  /opt/homebrew/opt/go/libexec/src/runtime/panic.go:770
    3  /Users/sasurai/Desktop/gno/gno/gnovm/pkg/gnolang/realm.go:502
    4  /Users/sasurai/Desktop/gno/gno/gnovm/pkg/gnolang/realm.go:498
    5  /Users/sasurai/Desktop/gno/gno/gnovm/pkg/gnolang/realm.go:498
    6  /Users/sasurai/Desktop/gno/gno/gnovm/pkg/gnolang/realm.go:498
    7  /Users/sasurai/Desktop/gno/gno/gnovm/pkg/gnolang/realm.go:498
    8  /Users/sasurai/Desktop/gno/gno/gnovm/pkg/gnolang/realm.go:498
    9  /Users/sasurai/Desktop/gno/gno/gnovm/pkg/gnolang/realm.go:498
   10  /Users/sasurai/Desktop/gno/gno/gnovm/pkg/gnolang/realm.go:462
   11  /Users/sasurai/Desktop/gno/gno/gnovm/pkg/gnolang/realm.go:319
   12  /Users/sasurai/Desktop/gno/gno/gnovm/pkg/gnolang/op_call.go:239
   13  /Users/sasurai/Desktop/gno/gno/gnovm/pkg/gnolang/machine.go:1158
   14  /Users/sasurai/Desktop/gno/gno/gnovm/pkg/gnolang/machine.go:734
   15  /Users/sasurai/Desktop/gno/gno/gno.land/pkg/sdk/vm/keeper.go:324
   16  /Users/sasurai/Desktop/gno/gno/gno.land/pkg/sdk/vm/handler.go:53
   17  /Users/sasurai/Desktop/gno/gno/gno.land/pkg/sdk/vm/handler.go:33
   18  /Users/sasurai/Desktop/gno/gno/tm2/pkg/sdk/baseapp.go:650
   19  /Users/sasurai/Desktop/gno/gno/tm2/pkg/sdk/baseapp.go:829
   20  /Users/sasurai/Desktop/gno/gno/tm2/pkg/sdk/helpers.go:17
   21  /Users/sasurai/Desktop/gno/gno/tm2/pkg/sdk/baseapp.go:418
   22  /Users/sasurai/Desktop/gno/gno/tm2/pkg/sdk/baseapp.go:391
   23  /Users/sasurai/Desktop/gno/gno/tm2/pkg/bft/abci/client/local_client.go:180
   24  /Users/sasurai/Desktop/gno/gno/tm2/pkg/bft/appconn/app_conn.go:144
   25  /Users/sasurai/Desktop/gno/gno/tm2/pkg/bft/rpc/core/abci.go:59
   26  /opt/homebrew/opt/go/libexec/src/reflect/value.go:596
   27  /opt/homebrew/opt/go/libexec/src/reflect/value.go:380
   28  /Users/sasurai/Desktop/gno/gno/tm2/pkg/bft/rpc/lib/server/handlers.go:185
   29  /Users/sasurai/Desktop/gno/gno/tm2/pkg/bft/rpc/lib/server/handlers.go:209
   30  /opt/homebrew/opt/go/libexec/src/net/http/server.go:2166
   31  /opt/homebrew/opt/go/libexec/src/net/http/server.go:2683
--= /Error =--
,events:[]
--= /Error =--

I tried isolating & replicating the issue with a simple txtar, but I didn't get lucky, as this test is passing. Any ideas?

@leohhhn
Copy link
Contributor Author

leohhhn commented Jun 3, 2024

Possibly related: #1543

@thinhnx-var
Copy link
Contributor

Since it can not be replicated, I wonder that did it happened with all the key of Blog tree or just randomly failed?

@leohhhn
Copy link
Contributor Author

leohhhn commented Jun 3, 2024

@thinhnx-var

It happens on every call to the function.

EDIT: It is a bit hard to replicate this - you can run gnodev, and use your address as the admin address in the gnoland/blog/admin.gno file. Then, you can use the gnoblog-cli from the blog repo to post a specific post to the local gnodev node, and then you can try calling the ModRemovePost() function in the gnoland/blog realm to see the issue.

Hopefully we will have better previews and ways to replicate these kind of things.

@deelawn
Copy link
Contributor

deelawn commented Jun 3, 2024

@leohhhn so the flow to reproduce is just add a post and then call ModRemovePost? I tried this and wasn't able to reproduce the issue. I did this running gnodev after replacing the admin address with the test1 address:

gnoblog-cli post posts/2022-05-02_peace/README.md -publish -key test1
gnokey maketx call -pkgpath "gno.land/r/gnoland/blog" -func "ModRemovePost" -gas-fee 1000000ugnot -gas-wanted 2000000 -send "" -broadcast -chainid "dev" -args "peace" -remote "tcp://127.0.0.1:26657" test1

@leohhhn
Copy link
Contributor Author

leohhhn commented Jun 4, 2024

@deelawn I tried it as well, and seems that gnodev has the transaction passing, but if you try to open up the blog on gnoweb, it errors with unexpected object with id b267d23c27c167591da35b2b4bd51435eddcd5eb:8. The Portal Loop however, still keeps breaking:

❯ gnokey maketx call -pkgpath "gno.land/r/gnoland/blog" -func "ModRemovePost" -gas-fee 1000000ugnot -gas-wanted 2000000 -send "" -broadcast -chainid "portal-loop" -args "the-gnome" -remote "https://rpc.gno.land:443" main
Enter password.
--= Error =--
Data: deleted descendants should not have a reference count of less than zero
Msg Traces:
    0  /Users/sasurai/Desktop/gno/gno/tm2/pkg/crypto/keys/client/maketx.go:213 - deliver transaction failed: log:msg:0,success:false,log:--= Error =--
Data: &errors.errorString{s:"deleted descendants should not have a reference count of less than zero"}
Msg Traces:
    0  /home/runner/work/gno/gno/gno.land/pkg/sdk/vm/keeper.go:318 - VM call panic: deleted descendants should not have a reference count of less than zero

Very weird.

@leohhhn leohhhn mentioned this issue Jun 5, 2024
7 tasks
leohhhn added a commit that referenced this issue Jun 5, 2024
<!-- please provide a detailed description of the changes made in this
pull request. -->

## Description

This PR provides a hotfix to the edit functionality in the blog package.
This is not the best solution, but due to the urgency of the requests, a
we cannot wait for a full refactor of the code, and there are still
issues with [AVL & GnoVM](#2266)
that are blocking the other proper workaround on this.

<details><summary>Contributors' checklist...</summary>

- [x] Added new tests, or not needed, or not feasible
- [x] Provided an example (e.g. screenshot) to aid review or the PR is
self-explanatory
- [x] Updated the official documentation or not needed
- [x] No breaking changes were made, or a `BREAKING CHANGE: xxx` message
was included in the description
- [x] Added references to related issues and PRs
- [ ] Provided any useful hints for running manual tests
- [ ] Added new benchmarks to [generated
graphs](https://gnoland.github.io/benchmarks), if any. More info
[here](https://github.com/gnolang/gno/blob/master/.benchmarks/README.md).
</details>
@Kouteki Kouteki added 🐞 bug Something isn't working 📦 🤖 gnovm Issues or PRs gnovm related labels Jun 7, 2024
@Kouteki Kouteki moved this from Triage to Backlog in 🧙‍♂️gno.land core team Jun 7, 2024
@Kouteki Kouteki added this to the 🚀 Mainnet launch milestone Oct 16, 2024
@omarsy omarsy linked a pull request Oct 21, 2024 that will close this issue
6 tasks
@Kouteki
Copy link
Contributor

Kouteki commented Oct 21, 2024

We'll assign this to @omarsy - have to add him to the team

@Kouteki Kouteki self-assigned this Oct 21, 2024
@Kouteki Kouteki moved this from Backlog to In Progress in 🧙‍♂️gno.land core team Oct 22, 2024
@Kouteki Kouteki assigned omarsy and unassigned Kouteki Oct 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐞 bug Something isn't working 📦 🤖 gnovm Issues or PRs gnovm related
Projects
Status: In Progress
Development

Successfully merging a pull request may close this issue.

6 participants