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

Ensure that memory expansion (grow_memory) aborts on failure #177

Open
axic opened this issue Feb 11, 2019 · 5 comments
Open

Ensure that memory expansion (grow_memory) aborts on failure #177

axic opened this issue Feb 11, 2019 · 5 comments
Labels

Comments

@axic
Copy link
Member

axic commented Feb 11, 2019

While the metering "spec" calls for prepending grow_memory with a gas check, the grow_memory opcode can still return a failure when enough gas is present, but not enough memory is available.

@d1m0 has suggested this is an issue.

I propose that instead of only injecting a gas check, grow_memory should be wrapped in a special check which causes a panic (or OOG) on the client.

I'm not sure what is the best behaviour (panic, OOG or something else?), but likely leaving it alone is wrong, because that gives an opportunity to the contract developer to check for the return value of grow_memory and create diverging behaviours.

@poemm
Copy link
Contributor

poemm commented Feb 11, 2019

We have discussed options of disallowing memory.grow, or bounding Wasm memory with gas charges.

The WebAssembly spec Section 7.2 Implementation Limitations lists many more things. There are some subtle things involving recursion.

@d1m0
Copy link

d1m0 commented Feb 11, 2019

In order to ensure non-determinism, I don't think we want OOG. It sounds strange that if we have sufficient gas for a given memory write, some clients may still throw an OOG because they are temporarily low on "physical" memory.

@poemm
Copy link
Contributor

poemm commented Feb 11, 2019

The option to bound Wasm memory with gas would involve the honest majority of miners keeping the gas limit low enough so that their clients can easily handle any amount of Wasm memory that is within the gas limit. EVM uses a similar mechanism.

@axic
Copy link
Member Author

axic commented Feb 11, 2019

It is not only the gas limit, but a combination of gas limit and gas price.

There are two ways to cause memory allocation systematically fail while still not running out of gas:
a) wrong gas cost (or not using the quadratic logic for memory usage)
b) wrong combination of gas price + gas limit

The variable in option b) can be influenced by the miners without a hard fork, while a) requires a hard fork.

@gcolvin
Copy link
Contributor

gcolvin commented Feb 15, 2019

I mentioned in the Gitter channel that long before most systems run out of virtual memory they will thrashing so badly as to be useless. So I'm not sure this problem that is going to happen in practice.

@axic axic added the metering label Mar 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants