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

How to handle [possible] negative VM refcounter #3328

Open
AnnaShaleva opened this issue Jun 11, 2024 · 1 comment · May be fixed by #3327
Open

How to handle [possible] negative VM refcounter #3328

AnnaShaleva opened this issue Jun 11, 2024 · 1 comment · May be fixed by #3327
Labels
Backlog Backlog issues and PRs Discussion Initial issue state - proposed but not yet accepted

Comments

@AnnaShaleva
Copy link
Member

Summary or problem description
Due to a possible bug in the node implementation, VM refcounter may become negative (e.g. like it happened in #3301 (comment)). We need to decide how to handle negative refcounter.

Quick suggestions were:

  1. FAULT the transaction immediately if refcounter becomes negative (implemented in [Neo VM Bug]Fix negative counter issue #3304).
  2. Stop the node immediately if refcounter becomes negative (implemented in [Neo VM Bug] Kill the process on negative reference counter #3324).
  3. Adjust the logic of references counting (implemented in [Neo VM Bug] Fix negative counter issue #3327).

We need to discuss and choose the best solution (or think about other possible solution) how to deal with negative refcounter.

Where in the software does this update applies to?

  • VM
@AnnaShaleva AnnaShaleva added the Discussion Initial issue state - proposed but not yet accepted label Jun 11, 2024
@shargon shargon linked a pull request Jun 11, 2024 that will close this issue
15 tasks
@shargon
Copy link
Member

shargon commented Jun 11, 2024

I think that 3 is the best solution for now, we need to check it deeply, but the main problem it was not DeepCopy, it was when is removed a CompundType that contains entries that was already present in the stack, the current implementation removed all of them, and it require a check for all the entries.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backlog Backlog issues and PRs Discussion Initial issue state - proposed but not yet accepted
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants