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

fix: EthAPI: use StateCompute for feeHistory; apply minimum gas premium #10413

Merged
merged 3 commits into from
Mar 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
135 changes: 115 additions & 20 deletions itests/eth_fee_history_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,96 +3,191 @@ package itests
import (
"context"
"encoding/json"
"sort"
"testing"
"time"

"github.com/stretchr/testify/require"

"github.com/filecoin-project/go-jsonrpc"
"github.com/filecoin-project/go-state-types/abi"

"github.com/filecoin-project/lotus/chain/types"
"github.com/filecoin-project/lotus/chain/types/ethtypes"
"github.com/filecoin-project/lotus/itests/kit"
"github.com/filecoin-project/lotus/lib/result"
"github.com/filecoin-project/lotus/node/impl/full"
)

// calculateExpectations calculates the expected number of items to be included in the response
// of eth_feeHistory. It takes care of null rounds by finding the closet tipset with height
// smaller than startHeight, and then looks back at requestAmount of items. It also considers
// scenarios where there are not enough items to look back.
func calculateExpectations(tsHeights []int, requestAmount, startHeight int) (count, oldestHeight int) {
latestIdx := sort.SearchInts(tsHeights, startHeight)
// SearchInts returns the index of the number that's larger than the target if the target
// doesn't exist. However, we're looking for the closet number that's smaller that the target
for tsHeights[latestIdx] > startHeight {
latestIdx--
}
cnt := requestAmount
oldestIdx := latestIdx - requestAmount + 1
if oldestIdx < 0 {
cnt = latestIdx + 1
oldestIdx = 0
}
return cnt, tsHeights[oldestIdx]
}

func TestEthFeeHistory(t *testing.T) {
require := require.New(t)

kit.QuietAllLogsExcept()

blockTime := 100 * time.Millisecond
client, _, ens := kit.EnsembleMinimal(t, kit.MockProofs(), kit.ThroughRPC())
ens.InterconnectAll().BeginMining(blockTime)

ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
defer cancel()

// Wait for the network to create 20 blocks
heads, err := client.ChainNotify(ctx)
require.NoError(err)

// Save the full view of the tipsets to calculate the answer when there are null rounds
tsHeights := []int{1}
Copy link
Member

Choose a reason for hiding this comment

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

Why does this need to be preloaded with 1?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems that ChainNotify doesn't send the tipset with height = 1

Copy link
Member

Choose a reason for hiding this comment

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

Hm, that suggests raciness. The ChainNotify is probably starting on tipset 2. Note that ChainNotify sends a "current" notification on subscribe, and the subscription might be starting on height 2. Do we absolutely need to use ChainNotify here? Alternatively you can walk the chain and populate tsHeights after the fact, i.e. after pausing the miner?

Copy link
Member

Choose a reason for hiding this comment

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

ChainNotify being called before the miner starts resolves my raciness concerns. Starting at 2 is probably an artifact from the testkit, and the way we set up miners.

go func() {
for chg := range heads {
for _, c := range chg {
tsHeights = append(tsHeights, int(c.Val.Height()))
Copy link
Member

Choose a reason for hiding this comment

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

Note that the first notification you receive will be the current tipset, i.e. c.Type will be store.HCCurrent.

}
}
}()

miner := ens.InterconnectAll().BeginMining(blockTime)

client.WaitTillChain(ctx, kit.HeightAtLeast(7))
miner[0].InjectNulls(abi.ChainEpoch(5))

// Wait for the network to create at least 20 tipsets
client.WaitTillChain(ctx, kit.HeightAtLeast(20))
ychiaoli18 marked this conversation as resolved.
Show resolved Hide resolved
for _, m := range miner {
m.Pause()
}

ch, err := client.ChainNotify(ctx)
require.NoError(err)

// Wait for 5 seconds of inactivity
func() {
for {
select {
case <-ch:
continue
case <-time.After(5 * time.Second):
return
}
}
}()

sort.Ints(tsHeights)

// because of the deferred execution, the last tipset is not executed yet,
// and the one before the last one is the last executed tipset,
// which corresponds to the "latest" tag in EthGetBlockByNumber
latestBlk := ethtypes.EthUint64(tsHeights[len(tsHeights)-2])
ychiaoli18 marked this conversation as resolved.
Show resolved Hide resolved
blk, err := client.EthGetBlockByNumber(ctx, "latest", false)
require.NoError(err)
require.Equal(blk.Number, latestBlk)

assertHistory := func(history *ethtypes.EthFeeHistory, requestAmount, startHeight int) {
amount, oldest := calculateExpectations(tsHeights, requestAmount, startHeight)
require.Equal(amount+1, len(history.BaseFeePerGas))
require.Equal(amount, len(history.GasUsedRatio))
require.Equal(ethtypes.EthUint64(oldest), history.OldestBlock)
}

history, err := client.EthFeeHistory(ctx, result.Wrap[jsonrpc.RawParams](
json.Marshal([]interface{}{5, "0x10"}),
).Assert(require.NoError))
require.NoError(err)
require.Equal(6, len(history.BaseFeePerGas))
require.Equal(5, len(history.GasUsedRatio))
require.Equal(ethtypes.EthUint64(16-5+1), history.OldestBlock)
assertHistory(&history, 5, 16)
require.Nil(history.Reward)

history, err = client.EthFeeHistory(ctx, result.Wrap[jsonrpc.RawParams](
json.Marshal([]interface{}{"5", "0x10"}),
).Assert(require.NoError))
require.NoError(err)
require.Equal(6, len(history.BaseFeePerGas))
require.Equal(5, len(history.GasUsedRatio))
require.Equal(ethtypes.EthUint64(16-5+1), history.OldestBlock)
assertHistory(&history, 5, 16)
require.Nil(history.Reward)

history, err = client.EthFeeHistory(ctx, result.Wrap[jsonrpc.RawParams](
json.Marshal([]interface{}{5, "latest"}),
).Assert(require.NoError))
require.NoError(err)
assertHistory(&history, 5, int(latestBlk))
require.Nil(history.Reward)

history, err = client.EthFeeHistory(ctx, result.Wrap[jsonrpc.RawParams](
json.Marshal([]interface{}{"0x10", "0x12"}),
).Assert(require.NoError))
require.NoError(err)
require.Equal(17, len(history.BaseFeePerGas))
require.Equal(16, len(history.GasUsedRatio))
require.Equal(ethtypes.EthUint64(18-16+1), history.OldestBlock)
assertHistory(&history, 16, 18)
require.Nil(history.Reward)

history, err = client.EthFeeHistory(ctx, result.Wrap[jsonrpc.RawParams](
json.Marshal([]interface{}{5, "0x10"}),
).Assert(require.NoError))
require.NoError(err)
require.Equal(6, len(history.BaseFeePerGas))
require.Equal(5, len(history.GasUsedRatio))
require.Equal(ethtypes.EthUint64(16-5+1), history.OldestBlock)
assertHistory(&history, 5, 16)
require.Nil(history.Reward)

history, err = client.EthFeeHistory(ctx, result.Wrap[jsonrpc.RawParams](
json.Marshal([]interface{}{5, "10"}),
).Assert(require.NoError))
require.NoError(err)
require.Equal(6, len(history.BaseFeePerGas))
require.Equal(5, len(history.GasUsedRatio))
require.Equal(ethtypes.EthUint64(10-5+1), history.OldestBlock)
assertHistory(&history, 5, 10)
require.Nil(history.Reward)

// test when the requested number of blocks is longer than chain length
history, err = client.EthFeeHistory(ctx, result.Wrap[jsonrpc.RawParams](
json.Marshal([]interface{}{"0x30", "latest"}),
).Assert(require.NoError))
require.NoError(err)
assertHistory(&history, 48, int(latestBlk))
require.Nil(history.Reward)

// test when the requested number of blocks is longer than chain length
history, err = client.EthFeeHistory(ctx, result.Wrap[jsonrpc.RawParams](
json.Marshal([]interface{}{"0x30", "10"}),
).Assert(require.NoError))
require.NoError(err)
assertHistory(&history, 48, 10)
require.Nil(history.Reward)

history, err = client.EthFeeHistory(ctx, result.Wrap[jsonrpc.RawParams](
json.Marshal([]interface{}{5, "10", &[]float64{25, 50, 75}}),
).Assert(require.NoError))
require.NoError(err)
require.Equal(6, len(history.BaseFeePerGas))
require.Equal(5, len(history.GasUsedRatio))
require.Equal(ethtypes.EthUint64(10-5+1), history.OldestBlock)
assertHistory(&history, 5, 10)
require.NotNil(history.Reward)
require.Equal(5, len(*history.Reward))
for _, arr := range *history.Reward {
require.Equal(3, len(arr))
for _, item := range arr {
require.Equal(ethtypes.EthBigInt(types.NewInt(full.MinGasPremium)), item)
}
}

history, err = client.EthFeeHistory(ctx, result.Wrap[jsonrpc.RawParams](
json.Marshal([]interface{}{1025, "10", &[]float64{25, 50, 75}}),
).Assert(require.NoError))
require.Error(err)

history, err = client.EthFeeHistory(ctx, result.Wrap[jsonrpc.RawParams](
json.Marshal([]interface{}{5, "10", &[]float64{75, 50}}),
).Assert(require.NoError))
require.Error(err)

history, err = client.EthFeeHistory(ctx, result.Wrap[jsonrpc.RawParams](
json.Marshal([]interface{}{5, "10", &[]float64{}}),
).Assert(require.NoError))
Expand Down
37 changes: 19 additions & 18 deletions node/impl/full/eth.go
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ func (a *EthModule) parseBlkParam(ctx context.Context, blkParam string) (tipset
if err != nil {
return nil, fmt.Errorf("cannot parse block number: %v", err)
}
ts, err := a.Chain.GetTipsetByHeight(ctx, abi.ChainEpoch(num), nil, false)
ts, err := a.Chain.GetTipsetByHeight(ctx, abi.ChainEpoch(num), nil, true)
raulk marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return nil, fmt.Errorf("cannot get tipset at height: %v", num)
}
Expand Down Expand Up @@ -681,11 +681,7 @@ func (a *EthModule) EthFeeHistory(ctx context.Context, p jsonrpc.RawParams) (eth
return ethtypes.EthFeeHistory{}, fmt.Errorf("bad block parameter %s: %s", params.NewestBlkNum, err)
}

// Deal with the case that the chain is shorter than the number of requested blocks.
oldestBlkHeight := uint64(1)
if abi.ChainEpoch(params.BlkCount) <= ts.Height() {
oldestBlkHeight = uint64(ts.Height()) - uint64(params.BlkCount) + 1
}

// NOTE: baseFeePerGas should include the next block after the newest of the returned range,
// because the next base fee can be inferred from the messages in the newest block.
Expand All @@ -695,29 +691,32 @@ func (a *EthModule) EthFeeHistory(ctx context.Context, p jsonrpc.RawParams) (eth
gasUsedRatioArray := []float64{}
rewardsArray := make([][]ethtypes.EthBigInt, 0)

for ts.Height() >= abi.ChainEpoch(oldestBlkHeight) {
// Unfortunately we need to rebuild the full message view so we can
// totalize gas used in the tipset.
msgs, err := a.Chain.MessagesForTipset(ctx, ts)
blocksIncluded := 0
for blocksIncluded < int(params.BlkCount) && ts.Height() > 0 {
compOutput, err := a.StateCompute(ctx, ts.Height(), nil, ts.Key())
raulk marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return ethtypes.EthFeeHistory{}, xerrors.Errorf("error loading messages for tipset: %v: %w", ts, err)
return ethtypes.EthFeeHistory{}, xerrors.Errorf("cannot lookup the status of tipset: %v: %w", ts, err)
}

txGasRewards := gasRewardSorter{}
for txIdx, msg := range msgs {
msgLookup, err := a.StateAPI.StateSearchMsg(ctx, types.EmptyTSK, msg.Cid(), api.LookbackNoLimit, false)
if err != nil || msgLookup == nil {
return ethtypes.EthFeeHistory{}, nil
for _, msg := range compOutput.Trace {
if msg.Msg.From == builtintypes.SystemActorAddr {
continue
}

smsgCid, err := getSignedMessage(ctx, a.Chain, msg.MsgCid)
if err != nil {
return ethtypes.EthFeeHistory{}, xerrors.Errorf("failed to get signed msg %s: %w", msg.MsgCid, err)
}

tx, err := newEthTxFromMessageLookup(ctx, msgLookup, txIdx, a.Chain, a.StateAPI)
tx, err := newEthTxFromSignedMessage(ctx, smsgCid, a.StateAPI)
if err != nil {
return ethtypes.EthFeeHistory{}, nil
return ethtypes.EthFeeHistory{}, err
}

txGasRewards = append(txGasRewards, gasRewardTuple{
reward: tx.Reward(ts.Blocks()[0].ParentBaseFee),
gas: uint64(msgLookup.Receipt.GasUsed),
gas: uint64(msg.MsgRct.GasUsed),
})
}

Expand All @@ -727,6 +726,8 @@ func (a *EthModule) EthFeeHistory(ctx context.Context, p jsonrpc.RawParams) (eth
baseFeeArray = append(baseFeeArray, ethtypes.EthBigInt(ts.Blocks()[0].ParentBaseFee))
gasUsedRatioArray = append(gasUsedRatioArray, float64(totalGasUsed)/float64(build.BlockGasLimit))
rewardsArray = append(rewardsArray, rewards)
oldestBlkHeight = uint64(ts.Height())
blocksIncluded++

parentTsKey := ts.Parents()
ts, err = a.Chain.LoadTipSet(ctx, parentTsKey)
Expand Down Expand Up @@ -2343,7 +2344,7 @@ func calculateRewardsAndGasUsed(rewardPercentiles []float64, txGasRewards gasRew

rewards := make([]ethtypes.EthBigInt, len(rewardPercentiles))
for i := range rewards {
rewards[i] = ethtypes.EthBigIntZero
rewards[i] = ethtypes.EthBigInt(types.NewInt(MinGasPremium))
}

if len(txGasRewards) == 0 {
Expand Down
2 changes: 1 addition & 1 deletion node/impl/full/eth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ func TestRewardPercentiles(t *testing.T) {
{
percentiles: []float64{25, 50, 75},
txGasRewards: []gasRewardTuple{},
answer: []int64{0, 0, 0},
answer: []int64{MinGasPremium, MinGasPremium, MinGasPremium},
},
{
percentiles: []float64{25, 50, 75, 100},
Expand Down