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

Include max memory size of GVM in genesis parameters? #761

Open
piux2 opened this issue Apr 19, 2023 · 3 comments
Open

Include max memory size of GVM in genesis parameters? #761

piux2 opened this issue Apr 19, 2023 · 3 comments
Labels
📦 🤖 gnovm Issues or PRs gnovm related 🌱 feature New update to Gno

Comments

@piux2
Copy link
Contributor

piux2 commented Apr 19, 2023

Description

Should we include a maximum memory size for GVM execution in the app's genesis state to ensure deterministic contract execution across nodes? We are open for discussion and propose option 5

The allocator (pkg/gnolang/alloc.go) manages the maximum memory size in bytes allocated to the VM. By default, there is no limit, but the maximum value can be set when we start the nodes from CLI.

Option 1: Keep the default setting. Since each machine has distinct physical memory limitations and Go environment configurations, the same contract execution may succeed or fail on different machines due to memory allocation variations in the Go runtime.

Option 2: Set the maximum value via the command line, as it is currently implemented. Different users may set different values, leading to inconsistent contract execution results across machines due to varying max alloc size settings.

Option 3: Set the maximum value in the config.toml. Users may still set different values or values could change between release, causing the same contract execution to have varying outcomes on different machines because of differing max alloc size settings.

Option 4: Hard-code the maximum value. If different values are used between releases and users run various node versions, the same contract execution may succeed or fail on different machines due to different max alloc size settings.

Option 5: Incorporate the GVM's maximum memory size as a genesis parameter, and verify the value when starting the node to ensure the machine has adequate memory and extra buffers to run the GVM. This ensures that a node joining the network has a deterministic maximum memory size allocation in GVM.

@r3v4s
Copy link
Contributor

r3v4s commented Apr 20, 2023

I think Option 1 ~ 3 is too risky since it can make different results.

For Option 4, we could limit node binary version just like cosmos does for gaiad.

Option 5 looks good but because of gno is open source, I'm wonder if there is any possibility that some (malicious) people will try to by pass check logic to break chain.

BTW, can you leave source code line number url for where Set the maximum value via the command line, as it is currently implemented. happens?

@moul
Copy link
Member

moul commented Apr 20, 2023

Yes, we need to fix this and other parameters to ensure determinism, maybe also the go runtime, arch?

I suggest trying to put such parameters on-chain, updatable by DAO (r/system/params). Add genesis line to set them up.

Edit: related discussion here: #871 (comment)

@piux2
Copy link
Contributor Author

piux2 commented Apr 21, 2023

I think Option 1 ~ 3 is too risky since it can make different results.

For Option 4, we could limit node binary version just like cosmos does for gaiad.

Option 5 looks good but because of gno is open source, I'm wonder if there is any possibility that some (malicious) people will try to by pass check logic to break chain.

BTW, can you leave source code line number url for where Set the maximum value via the command line, as it is currently implemented. happens?

@r3v4s, sorry for the confusion. The maxAlloc parameter isn't set via the gno.land node command line, but rather as a Machine option when creating the machine instance. In addition, we can provide the parameter for testing through cmd/gno/test.go by modifying the tests.TestMachine(testStore, stdout, "main") and adding the required parameters for maxAlloc.

Regarding option 5, its purpose is to avoid unintended or accidental changes that could cause discrepancies among nodes. As for malicious nodes, the execution result must undergo BFT consensus before being accepted by the network. The result will be as expected unless over 1/3 of voting power from the malicious nodes votes for different outcomes.

@ajnavarro ajnavarro added 🌱 feature New update to Gno 📦 🤖 gnovm Issues or PRs gnovm related labels May 19, 2023
@moul moul moved this to 🌟 Wanted for Launch in 🚀 The Launch [DEPRECATED] Sep 5, 2023
@moul moul moved this from 🌟 Wanted for Launch to 🚀 Needed for Launch in 🚀 The Launch [DEPRECATED] Sep 5, 2023
@moul moul added this to the 🚀 main.gno.land milestone Sep 6, 2023
@Kouteki Kouteki removed this from the 🚀 Mainnet launch milestone Oct 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📦 🤖 gnovm Issues or PRs gnovm related 🌱 feature New update to Gno
Projects
Status: 🚀 Needed for Launch
Development

No branches or pull requests

5 participants