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, les: fix sendTransaction API in the light mode #23215

Merged
merged 1 commit into from
Jul 15, 2021

Conversation

rjl493456442
Copy link
Member

This PR fixes the eth_sendTransaction API in the light mode. More detail in #23214

@@ -125,8 +127,18 @@ func (b *LesApiBackend) BlockByNumberOrHash(ctx context.Context, blockNrOrHash r
return nil, errors.New("invalid arguments; neither block nor hash specified")
}

func (b *LesApiBackend) PendingBlockAndReceipts() (*types.Block, types.Receipts) {
return nil, nil
func (b *LesApiBackend) PendingBlockAndReceipts(ctx context.Context) (*types.Block, types.Receipts, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this is necessary or a good idea. It's fine to revert the HeaderByNumber behavior to the original version (so that specifying pending returns latest) but this function explicitly targets the pending block and it's super weird to pass a context and return an error just so that it can return latest in light mode, then add a light flag to the oracle to ensure that we never actually call this function in light mode :)
Now I think the only fix needed is to revert HeaderByNumber behavior. The first version of the FeeHistory API function depended on this change but now it uses PendingBlockAndReceipts for pending (it never calls HeaderByNumber for pending block). So if we just leave PendingBlockAndReceipts as is (no need to add a light flag to the oracle either) and just revert HeaderByNumber then we should be fine and the code would be simpler.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I agree. The HeaderByNumber revert is enough for fixing. But when I implement this PR, I want to align the behavior that in the light mode we have no pending thing. It might be cleaner for the long term and whenever we introduce the pending things into the light client we can change it.

But I can use the simpler version for the fix if you think it's enough. But i think it's still needed to align the behavior.

@zsfelfoldi zsfelfoldi merged commit f05419f into ethereum:master Jul 15, 2021
sidhujag pushed a commit to sidhujag/go-ethereum that referenced this pull request Jul 28, 2021
atif-konasl pushed a commit to frozeman/pandora-execution-engine that referenced this pull request Oct 15, 2021
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.

2 participants