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 core errors in apply txs #1395

Merged

Conversation

Gustav-Simonsson
Copy link

Please merge this first: #1385

This changes the logic in core to propagate all errors except explicitly matched ones, currently only the OutOfGas error. This error had broken type matching due to being defined in two places, so this PR also refactors core errors into its own subpackage so it can be used both in core and in core/vm.

OOG error moved to core/vm on @fjl suggestion

@fjl
Copy link
Contributor

fjl commented Jul 3, 2015

I think the errors should stay in package core. errors is a a particulary bad package name
because it clashes with a prominent package in the standard library.

You can work around the import cycle by explicitly returning vm.OutOfGasError from StateTransition.UseGas. Additionally, I would recommend to define these kinds of 'empty struct' errors as global variables. This is a very common idiom.

package vm

import "errors"

var OutOfGas = errors.New("out of gas")

@Gustav-Simonsson Gustav-Simonsson force-pushed the fix_core_errors_in_apply_txs branch 2 times, most recently from 6b1f592 to 7c94dee Compare July 3, 2015 17:54
@@ -6,14 +6,18 @@ import (
"github.com/ethereum/go-ethereum/params"
)

type OutOfGasError struct{}
var OutOfGasError = &OutOfGasErr{Message: "Out of gas"}
Copy link
Contributor

Choose a reason for hiding this comment

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

Why can't this be errors.New("out of gas")?

Copy link
Author

Choose a reason for hiding this comment

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

For consistency with the other core errors, which makes use of struct type equality rather than variable equality. If we change this one we should change all, I think. @obscuren ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Only those that do not have variables

obscuren added a commit that referenced this pull request Jul 6, 2015
@obscuren obscuren merged commit 46e7c85 into ethereum:develop Jul 6, 2015
tony-ricciardi pushed a commit to tony-ricciardi/go-ethereum that referenced this pull request Jan 20, 2022
Description
Improve reliability from the ethstats module

The connectionWrapper was cherrypicked from go-ethereum upstream:
ethereum@82a9e11 (ethereum#21404)

Other changes
Validator injecting its version to the proxy stats chunk

Tested
Manually in a local testnet

Related issues
Fixes ethereum#1395
Fixes ethereum#1397
Backwards compatibility
Yes
maoueh pushed a commit to streamingfast/go-ethereum that referenced this pull request Mar 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants