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.

Sign up for GitHub

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

Jump to bottom

Revert "feat: fix GetBlock for null rounds by returning nil (#12529)" #12641

Merged
merged 1 commit into from
Oct 28, 2024
Merged

Revert "feat: fix GetBlock for null rounds by returning nil (#12529)" #12641

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
# UNRELEASED

## New features
- Update `EthGetBlockByNumber` to return a pointer to ethtypes.EthBlock or nil for null rounds. ([filecoin-project/lotus#12529](https://github.com/filecoin-project/lotus/pull/12529))
- Reduce size of embedded genesis CAR files by removing WASM actor blocks and compressing with zstd. This reduces the `lotus` binary size by approximately 10 MiB. ([filecoin-project/lotus#12439](https://github.com/filecoin-project/lotus/pull/12439))
- Add ChainSafe operated Calibration archival node to the bootstrap list ([filecoin-project/lotus#12517](https://github.com/filecoin-project/lotus/pull/12517))
- `lotus chain head` now supports a `--height` flag to print just the epoch number of the current chain head ([filecoin-project/lotus#12609](https://github.com/filecoin-project/lotus/pull/12609))
Expand Down
2 changes: 1 addition & 1 deletion api/api_full.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -775,7 +775,7 @@ type FullNode interface {
EthGetBlockTransactionCountByHash(ctx context.Context, blkHash ethtypes.EthHash) (ethtypes.EthUint64, error) //perm:read

EthGetBlockByHash(ctx context.Context, blkHash ethtypes.EthHash, fullTxInfo bool) (ethtypes.EthBlock, error) //perm:read
EthGetBlockByNumber(ctx context.Context, blkNum string, fullTxInfo bool) (*ethtypes.EthBlock, error) //perm:read
EthGetBlockByNumber(ctx context.Context, blkNum string, fullTxInfo bool) (ethtypes.EthBlock, error) //perm:read
EthGetTransactionByHash(ctx context.Context, txHash *ethtypes.EthHash) (*ethtypes.EthTx, error) //perm:read
EthGetTransactionByHashLimited(ctx context.Context, txHash *ethtypes.EthHash, limit abi.ChainEpoch) (*ethtypes.EthTx, error) //perm:read
EthGetTransactionHashByCid(ctx context.Context, cid cid.Cid) (*ethtypes.EthHash, error) //perm:read
Expand Down
2 changes: 1 addition & 1 deletion api/api_gateway.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ type Gateway interface {
EthGetBlockTransactionCountByNumber(ctx context.Context, blkNum ethtypes.EthUint64) (ethtypes.EthUint64, error)
EthGetBlockTransactionCountByHash(ctx context.Context, blkHash ethtypes.EthHash) (ethtypes.EthUint64, error)
EthGetBlockByHash(ctx context.Context, blkHash ethtypes.EthHash, fullTxInfo bool) (ethtypes.EthBlock, error)
EthGetBlockByNumber(ctx context.Context, blkNum string, fullTxInfo bool) (*ethtypes.EthBlock, error)
EthGetBlockByNumber(ctx context.Context, blkNum string, fullTxInfo bool) (ethtypes.EthBlock, error)
EthGetTransactionByHash(ctx context.Context, txHash *ethtypes.EthHash) (*ethtypes.EthTx, error)
EthGetTransactionByHashLimited(ctx context.Context, txHash *ethtypes.EthHash, limit abi.ChainEpoch) (*ethtypes.EthTx, error)
EthGetTransactionHashByCid(ctx context.Context, cid cid.Cid) (*ethtypes.EthHash, error)
Expand Down
4 changes: 2 additions & 2 deletions api/mocks/mock_full.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

20 changes: 10 additions & 10 deletions api/proxy_gen.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions build/openrpc/full.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2866,7 +2866,7 @@
},
{
"name": "Filecoin.EthGetBlockByNumber",
"description": "```go\nfunc (s *FullNodeStruct) EthGetBlockByNumber(p0 context.Context, p1 string, p2 bool) (*ethtypes.EthBlock, error) {\n\tif s.Internal.EthGetBlockByNumber == nil {\n\t\treturn nil, ErrNotSupported\n\t}\n\treturn s.Internal.EthGetBlockByNumber(p0, p1, p2)\n}\n```",
"description": "```go\nfunc (s *FullNodeStruct) EthGetBlockByNumber(p0 context.Context, p1 string, p2 bool) (ethtypes.EthBlock, error) {\n\tif s.Internal.EthGetBlockByNumber == nil {\n\t\treturn *new(ethtypes.EthBlock), ErrNotSupported\n\t}\n\treturn s.Internal.EthGetBlockByNumber(p0, p1, p2)\n}\n```",
"summary": "",
"paramStructure": "by-position",
"params": [
Expand Down Expand Up @@ -2902,8 +2902,8 @@
}
],
"result": {
"name": "*ethtypes.EthBlock",
"description": "*ethtypes.EthBlock",
"name": "ethtypes.EthBlock",
"description": "ethtypes.EthBlock",
"summary": "",
"schema": {
"examples": [
Expand Down
6 changes: 3 additions & 3 deletions build/openrpc/gateway.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2213,7 +2213,7 @@
},
{
"name": "Filecoin.EthGetBlockByNumber",
"description": "```go\nfunc (s *GatewayStruct) EthGetBlockByNumber(p0 context.Context, p1 string, p2 bool) (*ethtypes.EthBlock, error) {\n\tif s.Internal.EthGetBlockByNumber == nil {\n\t\treturn nil, ErrNotSupported\n\t}\n\treturn s.Internal.EthGetBlockByNumber(p0, p1, p2)\n}\n```",
"description": "```go\nfunc (s *GatewayStruct) EthGetBlockByNumber(p0 context.Context, p1 string, p2 bool) (ethtypes.EthBlock, error) {\n\tif s.Internal.EthGetBlockByNumber == nil {\n\t\treturn *new(ethtypes.EthBlock), ErrNotSupported\n\t}\n\treturn s.Internal.EthGetBlockByNumber(p0, p1, p2)\n}\n```",
"summary": "There are not yet any comments for this method.",
"paramStructure": "by-position",
"params": [
Expand Down Expand Up @@ -2249,8 +2249,8 @@
}
],
"result": {
"name": "*ethtypes.EthBlock",
"description": "*ethtypes.EthBlock",
"name": "ethtypes.EthBlock",
"description": "ethtypes.EthBlock",
"summary": "",
"schema": {
"examples": [
Expand Down
2 changes: 1 addition & 1 deletion gateway/node.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ type TargetAPI interface {
EthGetBlockTransactionCountByNumber(ctx context.Context, blkNum ethtypes.EthUint64) (ethtypes.EthUint64, error)
EthGetBlockTransactionCountByHash(ctx context.Context, blkHash ethtypes.EthHash) (ethtypes.EthUint64, error)
EthGetBlockByHash(ctx context.Context, blkHash ethtypes.EthHash, fullTxInfo bool) (ethtypes.EthBlock, error)
EthGetBlockByNumber(ctx context.Context, blkNum string, fullTxInfo bool) (*ethtypes.EthBlock, error)
EthGetBlockByNumber(ctx context.Context, blkNum string, fullTxInfo bool) (ethtypes.EthBlock, error)
EthGetTransactionByHashLimited(ctx context.Context, txHash *ethtypes.EthHash, limit abi.ChainEpoch) (*ethtypes.EthTx, error)
EthGetTransactionHashByCid(ctx context.Context, cid cid.Cid) (*ethtypes.EthHash, error)
EthGetMessageCidByTransactionHash(ctx context.Context, txHash *ethtypes.EthHash) (*cid.Cid, error)
Expand Down
6 changes: 3 additions & 3 deletions gateway/proxy_eth.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,13 +188,13 @@ func (gw *Node) EthGetBlockByHash(ctx context.Context, blkHash ethtypes.EthHash,
return gw.target.EthGetBlockByHash(ctx, blkHash, fullTxInfo)
}

func (gw *Node) EthGetBlockByNumber(ctx context.Context, blkNum string, fullTxInfo bool) (*ethtypes.EthBlock, error) {
func (gw *Node) EthGetBlockByNumber(ctx context.Context, blkNum string, fullTxInfo bool) (ethtypes.EthBlock, error) {
if err := gw.limit(ctx, stateRateLimitTokens); err != nil {
return nil, err
return ethtypes.EthBlock{}, err
}

if err := gw.checkBlkParam(ctx, blkNum, 0); err != nil {
return nil, err
return ethtypes.EthBlock{}, err
}

return gw.target.EthGetBlockByNumber(ctx, blkNum, fullTxInfo)
Expand Down
2 changes: 1 addition & 1 deletion itests/eth_api_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -408,7 +408,7 @@ func TestEthBlockNumberAliases(t *testing.T) {

head := client.WaitTillChain(ctx, kit.HeightAtLeast(policy.ChainFinality+100))
// latest should be head-1 (parents)
var latestEthBlk *ethtypes.EthBlock
var latestEthBlk ethtypes.EthBlock
for {
var err error
latestEthBlk, err = client.EVM().EthGetBlockByNumber(ctx, "latest", true)
Expand Down
14 changes: 4 additions & 10 deletions itests/eth_block_hash_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,24 +65,18 @@ func TestEthBlockHashesCorrect_MultiBlockTipset(t *testing.T) {
require.NoError(t, err)

ethBlockA, err := n2.EthGetBlockByNumber(ctx, hex, true)
// Cannot use static ErrFullRound error for comparison since it gets reserialized as a JSON RPC error.
if err != nil && strings.Contains(err.Error(), "null round") {
require.Less(t, ts.Height(), abi.ChainEpoch(i), "did not expect a tipset at epoch %d", i)
continue
}

// Check if the tipset height is less than or equal to the current epoch
require.LessOrEqual(t, ts.Height(), abi.ChainEpoch(i), "tipset height should not exceed the current epoch")

// Skip Null rounds
if ethBlockA == nil {
continue
}
require.NoError(t, err)
require.Equal(t, ts.Height(), abi.ChainEpoch(i), "expected a tipset at epoch %i", i)

ethBlockB, err := n2.EthGetBlockByHash(ctx, ethBlockA.Hash, true)
require.NoError(t, err)
require.NotNil(t, ethBlockB)

require.Equal(t, *ethBlockA, ethBlockB)
require.Equal(t, ethBlockA, ethBlockB)

numBlocks := len(ts.Blocks())
expGasLimit := ethtypes.EthUint64(int64(numBlocks) * buildconstants.BlockGasLimit)
Expand Down
4 changes: 2 additions & 2 deletions itests/eth_deploy_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ func TestDeployment(t *testing.T) {
blkNum := strconv.FormatInt(int64(*chainTx.BlockNumber), 10)
block2, err := client.EthGetBlockByNumber(ctx, blkNum, false)
require.Nil(t, err)
require.True(t, reflect.DeepEqual(block1, *block2))
require.True(t, reflect.DeepEqual(block1, block2))

// verify that the block contains full tx objects
block3, err := client.EthGetBlockByHash(ctx, *chainTx.BlockHash, true)
Expand Down Expand Up @@ -235,7 +235,7 @@ func TestDeployment(t *testing.T) {
// make sure the _full_ block got from EthGetBlockByNumber is the same
block4, err := client.EthGetBlockByNumber(ctx, blkNum, true)
require.Nil(t, err)
require.True(t, reflect.DeepEqual(block3, *block4))
require.True(t, reflect.DeepEqual(block3, block4))

// Verify that the deployer is now an account.
client.AssertActorType(ctx, deployer, manifest.EthAccountKey)
Expand Down
5 changes: 2 additions & 3 deletions itests/eth_transactions_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -454,9 +454,8 @@ func TestGetBlockByNumber(t *testing.T) {
}

// Fail when trying to fetch a null round.
block, err := client.EthGetBlockByNumber(ctx, (ethtypes.EthUint64(nullHeight)).Hex(), true)
require.NoError(t, err)
require.Nil(t, block)
_, err = client.EthGetBlockByNumber(ctx, (ethtypes.EthUint64(nullHeight)).Hex(), true)
require.Error(t, err)

// Fetch balance on a null round; should not fail and should return previous balance.
bal, err := client.EthGetBalance(ctx, ethAddr, ethtypes.NewEthBlockNumberOrHashFromNumber(ethtypes.EthUint64(nullHeight)))
Expand Down
8 changes: 3 additions & 5 deletions itests/fevm_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1396,9 +1396,8 @@ func TestEthGetBlockByNumber(t *testing.T) {
require.NotNil(t, specificBlock)

// Test getting a future block (should fail)
futureBlock, err := client.EthGetBlockByNumber(ctx, (latest + 10000).Hex(), true)
_, err = client.EthGetBlockByNumber(ctx, (latest + 10000).Hex(), true)
require.Error(t, err)
require.Nil(t, futureBlock)

// Inject 10 null rounds
bms[0].InjectNulls(10)
Expand Down Expand Up @@ -1427,9 +1426,8 @@ func TestEthGetBlockByNumber(t *testing.T) {
}

// Test getting a block for a null round
nullRoundBlock, err := client.EthGetBlockByNumber(ctx, (ethtypes.EthUint64(nullHeight)).Hex(), true)
require.NoError(t, err)
require.Nil(t, nullRoundBlock)
_, err = client.EthGetBlockByNumber(ctx, (ethtypes.EthUint64(nullHeight)).Hex(), true)
require.ErrorContains(t, err, "requested epoch was a null round")

// Test getting balance on a null round
bal, err := client.EthGetBalance(ctx, ethAddr, ethtypes.NewEthBlockNumberOrHashFromNumber(ethtypes.EthUint64(nullHeight)))
Expand Down
4 changes: 2 additions & 2 deletions node/impl/full/dummy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ func (e *EthModuleDummy) EthGetBlockByHash(ctx context.Context, blkHash ethtypes
return ethtypes.EthBlock{}, ErrModuleDisabled
}

func (e *EthModuleDummy) EthGetBlockByNumber(ctx context.Context, blkNum string, fullTxInfo bool) (*ethtypes.EthBlock, error) {
return nil, ErrModuleDisabled
func (e *EthModuleDummy) EthGetBlockByNumber(ctx context.Context, blkNum string, fullTxInfo bool) (ethtypes.EthBlock, error) {
return ethtypes.EthBlock{}, ErrModuleDisabled
}

func (e *EthModuleDummy) EthGetTransactionByHash(ctx context.Context, txHash *ethtypes.EthHash) (*ethtypes.EthTx, error) {
Expand Down
19 changes: 4 additions & 15 deletions node/impl/full/eth.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ type EthModuleAPI interface {
EthGetBlockTransactionCountByNumber(ctx context.Context, blkNum ethtypes.EthUint64) (ethtypes.EthUint64, error)
EthGetBlockTransactionCountByHash(ctx context.Context, blkHash ethtypes.EthHash) (ethtypes.EthUint64, error)
EthGetBlockByHash(ctx context.Context, blkHash ethtypes.EthHash, fullTxInfo bool) (ethtypes.EthBlock, error)
EthGetBlockByNumber(ctx context.Context, blkNum string, fullTxInfo bool) (*ethtypes.EthBlock, error)
EthGetBlockByNumber(ctx context.Context, blkNum string, fullTxInfo bool) (ethtypes.EthBlock, error)
EthGetTransactionByHash(ctx context.Context, txHash *ethtypes.EthHash) (*ethtypes.EthTx, error)
EthGetTransactionByHashLimited(ctx context.Context, txHash *ethtypes.EthHash, limit abi.ChainEpoch) (*ethtypes.EthTx, error)
EthGetMessageCidByTransactionHash(ctx context.Context, txHash *ethtypes.EthHash) (*cid.Cid, error)
Expand Down Expand Up @@ -341,23 +341,12 @@ func (a *EthModule) EthGetBlockByHash(ctx context.Context, blkHash ethtypes.EthH
return blk, nil
}

func (a *EthModule) EthGetBlockByNumber(ctx context.Context, blkParam string, fullTxInfo bool) (*ethtypes.EthBlock, error) {
// Get the tipset for the specified block parameter
func (a *EthModule) EthGetBlockByNumber(ctx context.Context, blkParam string, fullTxInfo bool) (ethtypes.EthBlock, error) {
ts, err := getTipsetByBlockNumber(ctx, a.Chain, blkParam, true)
if err != nil {
if err == ErrNullRound {
// Return nil for null rounds
return nil, nil
}
return nil, xerrors.Errorf("failed to get tipset: %w", err)
return ethtypes.EthBlock{}, err
}
// Create an Ethereum block from the Filecoin tipset
block, err := newEthBlockFromFilecoinTipSet(ctx, ts, fullTxInfo, a.Chain, a.StateAPI)
if err != nil {
return nil, xerrors.Errorf("failed to create Ethereum block: %w", err)
}

return &block, nil
return newEthBlockFromFilecoinTipSet(ctx, ts, fullTxInfo, a.Chain, a.StateAPI)
}

func (a *EthModule) EthGetTransactionByHash(ctx context.Context, txHash *ethtypes.EthHash) (*ethtypes.EthTx, error) {
Expand Down
Loading