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

Only one instance should exist per function call #325

Open
ureeves opened this issue Feb 10, 2024 · 0 comments · Fixed by #326
Open

Only one instance should exist per function call #325

ureeves opened this issue Feb 10, 2024 · 0 comments · Fixed by #326
Labels
fix:bug Something isn't working team:Core Low Level Core Development Team (Rust)

Comments

@ureeves
Copy link
Member

ureeves commented Feb 10, 2024

Describe the bug
Currently, when a contract is called multiple times during a call cycle, its instance is reused by all those calls. Even though this is deterministic, it leads to different behavior (different state root) than if this wasn't the case.

Furthermore, the logic of the reusing has some edge cases under which it will cause some bad dereferences, and removing it in favor of an implementation closer to a stack might be beneficial.

Also the instantiation of an Instance designed to be cheap, so we shouldn't be shy with doing so.

Expected behaviour
There should be one instance per contract function call.

Additional context
One of the bad dereferences was triggered during the development of dusk-network/rusk#1247

@ureeves ureeves added fix:bug Something isn't working team:Core Low Level Core Development Team (Rust) labels Feb 10, 2024
ureeves pushed a commit that referenced this issue Feb 10, 2024
Changes the instance reclamation code to allow for exactly one module
instance per contract function call. This changes the way in which the
node processes the state, leading to diverging state roots on erroring
calls.

Simplifies the instance code by using an atomic integer to construct an
atomically reference counted pointer, which then gets shared with the
environment. This fixes some bad dereferences occuring under certain
circumstances.

Resolves: #325
ureeves pushed a commit that referenced this issue Feb 12, 2024
Changes the instance reclamation code to allow for exactly one module
instance per contract function call. This changes the way in which the
node processes the state, leading to diverging state roots on erroring
calls.

Simplifies the instance code by using a boolean to appropriately drop
the instance. This allows the data in the struct to be passed around
the `Env`, whilst fixing some bad dereferences previously occuring under
certain situations.

Resolves: #325
ureeves pushed a commit that referenced this issue Feb 12, 2024
Changes the instance reclamation code to allow for exactly one module
instance per contract function call. This changes the way in which the
node processes the state, leading to diverging state roots on errorring
calls.

Simplifies the instance code by using a boolean to appropriately drop
the instance. This allows the data in the struct to be passed around
the `Env`, whilst fixing some bad dereferences previously occurring
under certain situations.

Resolves: #325
ureeves pushed a commit that referenced this issue Feb 12, 2024
Changes the instance reclamation code to allow for exactly one module
instance per contract function call. This changes the way in which the
node processes the state, leading to diverging state roots on errorring
calls.

Simplifies the instance code by using a boolean to appropriately drop
the instance. This allows the data in the struct to be passed around
the `Env`, whilst fixing some bad dereferences previously occurring
under certain situations.

Resolves: #325
@ureeves ureeves reopened this Feb 19, 2024
@HDauven HDauven added this to the Mainnet milestone Apr 10, 2024
@HDauven HDauven removed this from the Mainnet milestone Jul 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix:bug Something isn't working team:Core Low Level Core Development Team (Rust)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants