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

eth: remove check for tdd reached on pos api block tags #27799

Merged
merged 2 commits into from
Aug 26, 2023

Conversation

lightclient
Copy link
Member

We're dealing with a bit of a problem over in rpctestgen where we would like to generate some rpc test interactions using the safe and finalized block tags, but it is not currently possible without making some Engine API requests.

This is a relatively specialized use case, but the error returned is sort of incorrect here when a genesis has terminalTotalDifficultyPassed set in genesis: 'xxxxx' tag not supported on pre-merge network. Now it will return nil if the network is pre-merge or the network is post-merge but we haven't determined which block is safe / final yet. This behavior seems more correct.

Copy link
Contributor

@holiman holiman left a comment

Choose a reason for hiding this comment

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

LGTM, makes sense that the rpc layer just defers to blockchain about whether to return a block or an error

@holiman holiman added this to the 1.12.2 milestone Aug 10, 2023
@fjl fjl modified the milestones: 1.12.2, 1.13.0 Aug 11, 2023
@holiman holiman merged commit 3a662d4 into ethereum:master Aug 26, 2023
1 check passed
Copy link

@Deacon77 Deacon77 left a comment

Choose a reason for hiding this comment

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

I'm learning and new! If I made mistakes! Forgive me! I'll get better 3a662d4

@@ -78,24 +78,18 @@ func (b *EthAPIBackend) HeaderByNumber(ctx context.Context, number rpc.BlockNumb
return b.eth.blockchain.CurrentBlock(), nil
}
if number == rpc.FinalizedBlockNumber {
if !b.eth.Merger().TDDReached() {

This comment was marked as spam.

@@ -78,24 +78,18 @@ func (b *EthAPIBackend) HeaderByNumber(ctx context.Context, number rpc.BlockNumb
return b.eth.blockchain.CurrentBlock(), nil
}
if number == rpc.FinalizedBlockNumber {
if !b.eth.Merger().TDDReached() {
return nil, errors.New("'finalized' tag not supported on pre-merge network")

This comment was marked as spam.

@@ -78,24 +78,18 @@ func (b *EthAPIBackend) HeaderByNumber(ctx context.Context, number rpc.BlockNumb
return b.eth.blockchain.CurrentBlock(), nil
}
if number == rpc.FinalizedBlockNumber {
if !b.eth.Merger().TDDReached() {
return nil, errors.New("'finalized' tag not supported on pre-merge network")

This comment was marked as spam.

@@ -78,24 +78,18 @@ func (b *EthAPIBackend) HeaderByNumber(ctx context.Context, number rpc.BlockNumb
return b.eth.blockchain.CurrentBlock(), nil
}
if number == rpc.FinalizedBlockNumber {
if !b.eth.Merger().TDDReached() {
return nil, errors.New("'finalized' tag not supported on pre-merge network")

This comment was marked as spam.

@@ -78,24 +78,18 @@ func (b *EthAPIBackend) HeaderByNumber(ctx context.Context, number rpc.BlockNumb
return b.eth.blockchain.CurrentBlock(), nil
}
if number == rpc.FinalizedBlockNumber {
if !b.eth.Merger().TDDReached() {
return nil, errors.New("'finalized' tag not supported on pre-merge network")

This comment was marked as spam.

@@ -136,19 +130,13 @@ func (b *EthAPIBackend) BlockByNumber(ctx context.Context, number rpc.BlockNumbe
return b.eth.blockchain.GetBlock(header.Hash(), header.Number.Uint64()), nil
}
if number == rpc.FinalizedBlockNumber {
if !b.eth.Merger().TDDReached() {

This comment was marked as spam.

@@ -136,19 +130,13 @@ func (b *EthAPIBackend) BlockByNumber(ctx context.Context, number rpc.BlockNumbe
return b.eth.blockchain.GetBlock(header.Hash(), header.Number.Uint64()), nil
}
if number == rpc.FinalizedBlockNumber {
if !b.eth.Merger().TDDReached() {

This comment was marked as spam.

@@ -136,19 +130,13 @@ func (b *EthAPIBackend) BlockByNumber(ctx context.Context, number rpc.BlockNumbe
return b.eth.blockchain.GetBlock(header.Hash(), header.Number.Uint64()), nil
}
if number == rpc.FinalizedBlockNumber {
if !b.eth.Merger().TDDReached() {
return nil, errors.New("'finalized' tag not supported on pre-merge network")

This comment was marked as spam.

@@ -136,19 +130,13 @@ func (b *EthAPIBackend) BlockByNumber(ctx context.Context, number rpc.BlockNumbe
return b.eth.blockchain.GetBlock(header.Hash(), header.Number.Uint64()), nil
}
if number == rpc.FinalizedBlockNumber {
if !b.eth.Merger().TDDReached() {
return nil, errors.New("'finalized' tag not supported on pre-merge network")
}

This comment was marked as spam.

@@ -136,19 +130,13 @@ func (b *EthAPIBackend) BlockByNumber(ctx context.Context, number rpc.BlockNumbe
return b.eth.blockchain.GetBlock(header.Hash(), header.Number.Uint64()), nil
}
if number == rpc.FinalizedBlockNumber {
if !b.eth.Merger().TDDReached() {
return nil, errors.New("'finalized' tag not supported on pre-merge network")

This comment was marked as spam.

devopsbo3 pushed a commit to HorizenOfficial/go-ethereum that referenced this pull request Nov 10, 2023
This change defers to the blockchain for in what circumstances to return error, instead of handling many error-cases in the api backend.
devopsbo3 added a commit to HorizenOfficial/go-ethereum that referenced this pull request Nov 10, 2023
devopsbo3 added a commit to HorizenOfficial/go-ethereum that referenced this pull request Nov 10, 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.

5 participants