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

Fix homestead [WIP] #125

Merged
merged 6 commits into from
Jun 23, 2017
Merged

Fix homestead [WIP] #125

merged 6 commits into from
Jun 23, 2017

Conversation

cdetrio
Copy link
Contributor

@cdetrio cdetrio commented Jun 8, 2017

The first commit prevents runState.gasLimit from going negative and failing the test case call_OOG_additionalGasCosts1 under Homestead.

The second commit fixes the shallowStack_d10g0v0_Homestead test case, which executes the bytecode 0x60010b600055:

[1] PUSH1 0x01 
[2] SIGNEXTEND 
[4] PUSH1 0x00 
[5] SSTORE 

and should throw an OOG due to a stack underflow.

The third commit fixes some failing edge cases in the stackOverflow tests.

@cdetrio cdetrio changed the title Fix homestead Fix homestead [WIP] Jun 8, 2017
try {
subGas(runState, new BN(fees.callNewAccountGas.v))
} catch (e) {
done(e.error)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be within a if (homestead) check?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the rule that a 0-value call creates a new account (and pays the new account gas fee) was there in frontier.

Copy link
Member

@axic axic Jun 8, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, the commit log was a bit misleading. It fixes a bug when there wasn't sufficient gas left.

@@ -12,7 +12,7 @@ const codes = {
0x08: ['ADDMOD', 8, 3, 1, false],
0x09: ['MULMOD', 8, 3, 1, false],
0x0a: ['EXP', 10, 2, 1, false],
0x0b: ['SIGNEXTEND', 5, 1, 1, false],
0x0b: ['SIGNEXTEND', 5, 2, 1, false],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you review the rest of the opcodes? Others might have bugs :)

lib/runCode.js Outdated
@@ -131,6 +128,10 @@ module.exports = function (opts, cb) {
return done(ERROR.STACK_UNDERFLOW)
}

if (runState.stack.length - opInfo.in + opInfo.out > 1024) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you enclose the left side in parentheses?

@axic
Copy link
Member

axic commented Jun 8, 2017

Note that #84 will also be needed to complete homestead.

@axic
Copy link
Member

axic commented Jun 8, 2017

There is another bug, CALLCODE doesn't check for gas underflow for the stipend. Search for isub.

@wanderer wanderer merged commit cbf6ba1 into ethereumjs:master Jun 23, 2017
holgerd77 added a commit that referenced this pull request Dec 1, 2020
Added eth_getBlockTransactionCountByHash RPC method
holgerd77 added a commit that referenced this pull request Dec 14, 2020
…refactoring

Base trie code documentation and function clustering
holgerd77 added a commit that referenced this pull request Feb 15, 2021
holgerd77 added a commit that referenced this pull request May 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants