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

feat: return an error for null rounds from RPC methods #12655

Merged
merged 11 commits into from
Oct 31, 2024

Conversation

virajbhartiya
Copy link
Member

Related Issues

Fixes #12633

Checklist

Before you mark the PR ready for review, please make sure that:

@virajbhartiya virajbhartiya requested a review from rvagg October 29, 2024 03:39
@virajbhartiya
Copy link
Member Author

I have referenced the following for injecting null rounds in the itests,

bms[0].InjectNulls(10)
// wait until we produce blocks again
tctx, cancel := context.WithTimeout(ctx, 30*time.Second)
defer cancel()
ch, err := client.ChainNotify(tctx)
require.NoError(t, err)
<-ch // current
hc := <-ch // wait for next block
require.Equal(t, store.HCApply, hc[0].Type)
afterNullHeight := hc[0].Val.Height()
nullHeight := afterNullHeight - 1
for nullHeight > 0 {
ts, err := client.ChainGetTipSetByHeight(ctx, nullHeight, types.EmptyTSK)
require.NoError(t, err)
if ts.Height() == nullHeight {
nullHeight--
} else {
break
}
}
but getTipsetByBlockNumber is always resulting in requested a future epoch (beyond 'latest') and not returning the ErrNullRound error.

itests/fevm_test.go Outdated Show resolved Hide resolved
itests/fevm_test.go Outdated Show resolved Hide resolved
itests/fevm_test.go Outdated Show resolved Hide resolved
itests/fevm_test.go Outdated Show resolved Hide resolved
itests/fevm_test.go Outdated Show resolved Hide resolved
itests/fevm_test.go Outdated Show resolved Hide resolved
@virajbhartiya
Copy link
Member Author

@rvagg the TestEthFeeHistory was failing because of now inclusion of null rounds, I have made most of the changes and the refactored calculateExpectations but it is still failing and I am unable to figure out why. Also addressed other comments

node/impl/full/eth_utils.go Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
Copy link
Member

@rvagg rvagg left a comment

Choose a reason for hiding this comment

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

👌 great work

@rvagg rvagg merged commit 6a70c6b into filecoin-project:master Oct 31, 2024
81 checks passed
@virajbhartiya virajbhartiya deleted the err-null-round branch November 12, 2024 02:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ☑️ Done (Archive)
Development

Successfully merging this pull request may close these issues.

eth_getBlockByNumber should return error for null round
2 participants