chore(core): disable 4844 transactions by default #28
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
leeren
requested review from
LeoHChen,
AndyBoWu,
edisonz0718,
jdubpark and
limengformal
leeren
force-pushed
the
chore/disable-blob-transactions
branch
from
5756385
to
d4991ee
Compare
LeoHChen
reviewed
Oct 9, 2024
limengformal
approved these changes
Oct 9, 2024
Comment on lines
+57
to
+58
| if !opts.Config.Is4844Enabled() && tx.Type() == types.BlobTxType { | ||
| return core.ErrTxTypeNotSupported |
There was a problem hiding this comment.
I guess this is the part where blob tx is disabled and error is printed out without panic?
limengformal
requested changes
Oct 9, 2024
cmd/geth/config.go
Outdated
| @@ -183,6 +183,11 @@ func makeFullNode(ctx *cli.Context) *node.Node { | |||
| cfg.Eth.OverrideStoryNostoi = &v | |||
| } | |||
|
|
|||
| if ctx.IsSet(utils.Override4844Flag.Name) { | |||
There was a problem hiding this comment.
Probably better to just completely disable blob tx without having an option to enable it. We can enable later by changing the code. We don't want nodes to have different behaviors
There was a problem hiding this comment.
removed the flags and overrides
limengformal
reviewed
Oct 9, 2024
Comment on lines
+823
to
+826
| // Is4844Enabled checks whether blob transactions are supported. | ||
| func (c *ChainConfig) Is4844Enabled() bool { | ||
| return false | ||
| } |
There was a problem hiding this comment.
Can we directly remove it? We want to minimize the changes in geth repo so it is easier for us to incorporate changes from geth main repo.
limengformal
approved these changes
Oct 9, 2024
leeren
added a commit
that referenced
this pull request
Oct 15, 2024
* chore(core): disable 4844 support by default
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Ensures that 4844 transactions are by default not supported, with the ability to override this setting in the future.
fixes #16