-
Notifications
You must be signed in to change notification settings - Fork 21.2k
Revert changes for blob #32463
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
Revert changes for blob #32463
Conversation
Sorry I don't get it. What's the problem with it? |
@rjl493456442 This is related with: |
Blob Transaction is introduced since Cancun fork, it makes no sense to run the eviction before Any particular reasons for this change? Or can you elaborate the situation in your case? Did you |
If you go to the git blame you will see that you have introduced an if
statement that optimize the process to not call the function if not cancun.
The clients without this change work fine, clients after your changes
stopped working.
I believe that function is imprecise and is not triggered when it needs to
be triggered. What I suggest is to revert to the state, where it is known
to be working.
…On Tue, 19 Aug 2025, 03:46 rjl493456442, ***@***.***> wrote:
*rjl493456442* left a comment (ethereum/go-ethereum#32463)
<#32463 (comment)>
Blob Transaction is introduced since Cancun fork, it makes no sense to run
the eviction before
the cancun fork (impossible to include txs before that).
Any particular reasons for this change? Or can you elaborate the situation
in your case? Did you
activate the blob txs before the cancun?
—
Reply to this email directly, view it on GitHub
<#32463 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AGKQ6MNKY3ZAISYXWVPP3VL3OJ6XPAVCNFSM6AAAAACEEZLSN2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTCOJYHE2DGOJWGQ>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Please elaborate on the situation where you believe it should be triggered As I mentioned, blob transactions have been supported since the Cancun |
Cmon look at arguments that you pass for is cancun function.
…On Tue, 19 Aug 2025, 08:15 rjl493456442, ***@***.***> wrote:
*rjl493456442* left a comment (ethereum/go-ethereum#32463)
<#32463 (comment)>
I believe that function is imprecise and is not triggered when it needs to
be triggered.
Please elaborate on the situation where you believe it should be triggered
but currently is not.
As I mentioned, blob transactions have been supported since the Cancun
fork, so it is unnecessary to finalize the limbo state prior to Cancun.
—
Reply to this email directly, view it on GitHub
<#32463 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AGKQ6MOEZ4FSPJVAC3KQJUL3OK6GTAVCNFSM6AAAAACEEZLSN2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTCOJZGM2DOMRYHE>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Sorry for the noise, but you are right. I wrote down the tests and only issue I see is that the error is a noise, which could be mitigated if there would be a check if |
so the bad block error doesn't originate from the function you mentioned? |
I think its correlation not causation.
Perhaps checking for nil would clear some noise in the logs
…On Wed, 20 Aug 2025, 05:06 Muhammad Yasir, ***@***.***> wrote:
*SyedMuhamadYasir* left a comment (ethereum/go-ethereum#32463)
<#32463 (comment)>
I believe that function is imprecise and is not triggered when it needs to
be triggered.
Please elaborate on the situation where you believe it should be triggered
but currently is not.
As I mentioned, blob transactions have been supported since the Cancun
fork, so it is unnecessary to finalize the limbo state prior to Cancun.
Sorry for the noise, but you are right. I wrote down the tests and only
issue I see is that the error is a noise, which could be mitigated if there
would be a check if p.chain.CurrentFinalBlock() is not nil. The nil
causes the noise.
so the bad block error doesn't originate from the function you mentioned?
—
Reply to this email directly, view it on GitHub
<#32463 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AGKQ6MPR7BWH53MLVSQRU3D3OPQ2DAVCNFSM6AAAAACEEZLSN2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTEMBUGAYTCMRSGM>
.
You are receiving this because you modified the open/close state.Message
ID: ***@***.***>
|
Although I think tests that check the if condition are valuable, perhaps
you can cherry pick them?
…On Wed, 20 Aug 2025, 08:32 Błażej Krząkała, ***@***.***> wrote:
I think its correlation not causation.
Perhaps checking for nil would clear some noise in the logs
On Wed, 20 Aug 2025, 05:06 Muhammad Yasir, ***@***.***>
wrote:
> *SyedMuhamadYasir* left a comment (ethereum/go-ethereum#32463)
> <#32463 (comment)>
>
> I believe that function is imprecise and is not triggered when it needs to
> be triggered.
>
> Please elaborate on the situation where you believe it should be
> triggered but currently is not.
> As I mentioned, blob transactions have been supported since the Cancun
> fork, so it is unnecessary to finalize the limbo state prior to Cancun.
>
> Sorry for the noise, but you are right. I wrote down the tests and only
> issue I see is that the error is a noise, which could be mitigated if there
> would be a check if p.chain.CurrentFinalBlock() is not nil. The nil
> causes the noise.
>
> so the bad block error doesn't originate from the function you mentioned?
>
> —
> Reply to this email directly, view it on GitHub
> <#32463 (comment)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AGKQ6MPR7BWH53MLVSQRU3D3OPQ2DAVCNFSM6AAAAACEEZLSN2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTEMBUGAYTCMRSGM>
> .
> You are receiving this because you modified the open/close state.Message
> ID: ***@***.***>
>
|
It causes drop of participation rate, I believe that those changes were inserted prematurely