Skip to content

Commit

Permalink
refactorings
Browse files Browse the repository at this point in the history
  • Loading branch information
chmllr committed Feb 21, 2025
1 parent 293cb8f commit 24b6ad1
Show file tree
Hide file tree
Showing 8 changed files with 63 additions and 52 deletions.
14 changes: 8 additions & 6 deletions octane/evmengine/keeper/abci_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ import (
var zeroAddr common.Address
var zeroHash common.Hash

const maxWithdrawalsPerBlock uint64 = 32

func TestKeeper_PrepareProposal(t *testing.T) {
t.Parallel()

Expand Down Expand Up @@ -129,7 +131,7 @@ func TestKeeper_PrepareProposal(t *testing.T) {
address: tutil.RandomAddress(),
}
frp := newRandomFeeRecipientProvider()
k, err := NewKeeper(cdc, storeService, &tt.mockEngine, txConfig, ap, frp)
k, err := NewKeeper(cdc, storeService, &tt.mockEngine, txConfig, ap, frp, maxWithdrawalsPerBlock)
require.NoError(t, err)
populateGenesisHead(ctx, t, k)

Expand Down Expand Up @@ -157,7 +159,7 @@ func TestKeeper_PrepareProposal(t *testing.T) {
address: common.BytesToAddress([]byte("test")),
}
frp := newRandomFeeRecipientProvider()
keeper, err := NewKeeper(cdc, storeService, &mockEngine, txConfig, ap, frp, mockEventProc{})
keeper, err := NewKeeper(cdc, storeService, &mockEngine, txConfig, ap, frp, maxWithdrawalsPerBlock, mockEventProc{})
require.NoError(t, err)
keeper.SetVoteProvider(mockVEProvider{})
populateGenesisHead(ctx, t, keeper)
Expand Down Expand Up @@ -220,7 +222,7 @@ func TestKeeper_PrepareProposal(t *testing.T) {
address: common.BytesToAddress([]byte("test")),
}
frp := newRandomFeeRecipientProvider()
keeper, err := NewKeeper(cdc, storeService, &mockEngine, txConfig, ap, frp, mockEventProc{})
keeper, err := NewKeeper(cdc, storeService, &mockEngine, txConfig, ap, frp, maxWithdrawalsPerBlock, mockEventProc{})
require.NoError(t, err)
keeper.SetVoteProvider(mockVEProvider{})
populateGenesisHead(ctx, t, keeper)
Expand Down Expand Up @@ -291,7 +293,7 @@ func TestKeeper_PrepareProposal(t *testing.T) {
address: common.BytesToAddress([]byte("test")),
}
frp := newRandomFeeRecipientProvider()
keeper, err := NewKeeper(cdc, storeService, &mockEngine, txConfig, ap, frp, mockEventProc{})
keeper, err := NewKeeper(cdc, storeService, &mockEngine, txConfig, ap, frp, maxWithdrawalsPerBlock, mockEventProc{})
require.NoError(t, err)
keeper.SetVoteProvider(mockVEProvider{})
populateGenesisHead(ctx, t, keeper)
Expand Down Expand Up @@ -348,7 +350,7 @@ func TestKeeper_PrepareProposal(t *testing.T) {
address: common.BytesToAddress([]byte("test")),
}
frp := newRandomFeeRecipientProvider()
keeper, err := NewKeeper(cdc, storeService, &mockEngine, txConfig, ap, frp, mockEventProc{})
keeper, err := NewKeeper(cdc, storeService, &mockEngine, txConfig, ap, frp, maxWithdrawalsPerBlock, mockEventProc{})
require.NoError(t, err)
keeper.SetVoteProvider(mockVEProvider{})
populateGenesisHead(ctx, t, keeper)
Expand Down Expand Up @@ -407,7 +409,7 @@ func TestOptimistic(t *testing.T) {
}

frp := newRandomFeeRecipientProvider()
keeper, err := NewKeeper(cdc, storeService, &mockEngine, txConfig, ap, frp, mockEventProc{})
keeper, err := NewKeeper(cdc, storeService, &mockEngine, txConfig, ap, frp, maxWithdrawalsPerBlock, mockEventProc{})
require.NoError(t, err)
keeper.SetVoteProvider(mockVEProvider{})
keeper.SetCometAPI(cmtAPI)
Expand Down
22 changes: 10 additions & 12 deletions octane/evmengine/keeper/db.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package keeper
import (
"bytes"
"context"
"sort"

"github.com/omni-network/omni/lib/cast"
"github.com/omni-network/omni/lib/errors"
Expand Down Expand Up @@ -115,8 +114,11 @@ func (k *Keeper) listWithdrawalsByAddress(ctx context.Context, withdrawalAddr co
return withdrawals, nil
}

// EligibleWithdrawals returns all withdrawals created below the specified height, sorted by the created height, limited by the provided count.
func (k *Keeper) EligibleWithdrawals(ctx context.Context, height uint64, limit int) ([]*Withdrawal, error) {
// EligibleWithdrawals returns all withdrawals created below the specified height,
// sorted by the id (oldest to newest), limited by the provided count.
func (k *Keeper) EligibleWithdrawals(ctx context.Context) ([]*Withdrawal, error) {
height := sdk.UnwrapSDKContext(ctx).BlockHeight()
// Note: items are ordered by the id in ascending order (oldest to newest).
iter, err := k.withdrawalTable.List(ctx, WithdrawalPrimaryKey{})
if err != nil {
return nil, errors.Wrap(err, "list withdrawals")
Expand All @@ -130,17 +132,13 @@ func (k *Keeper) EligibleWithdrawals(ctx context.Context, height uint64, limit i
return nil, errors.Wrap(err, "get withdrawal")
}

if val.GetCreatedHeight() < height {
if val.GetCreatedHeight() < uint64(height) {
withdrawals = append(withdrawals, val)
}
}

sort.Slice(withdrawals, func(i, j int) bool {
return withdrawals[i].GetCreatedHeight() < withdrawals[j].GetCreatedHeight()
})

if len(withdrawals) > limit {
withdrawals = withdrawals[:limit]
if uint64(len(withdrawals)) == k.maxWithdrawalsPerBlock {
break
}
}
}

return withdrawals, nil
Expand Down
22 changes: 13 additions & 9 deletions octane/evmengine/keeper/db_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ func TestKeeper_withdrawalsPersistence(t *testing.T) {
}
frp := newRandomFeeRecipientProvider()
evmLogProc := mockEventProc{deliverErr: errors.New("test error")}
keeper, err := NewKeeper(cdc, storeService, &mockEngine, txConfig, ap, frp, evmLogProc)
maxWithdrawalsPerBlock := uint64(4)
keeper, err := NewKeeper(cdc, storeService, &mockEngine, txConfig, ap, frp, maxWithdrawalsPerBlock, evmLogProc)
require.NoError(t, err)

addr1 := tutil.RandomAddress()
Expand Down Expand Up @@ -94,28 +95,30 @@ func TestKeeper_withdrawalsPersistence(t *testing.T) {
require.NoError(t, err)
require.Empty(t, withdrawalsByAddr)

limit := 4
// make sure we have no withdrawals below and at height 1
withdrawalsByHeight, err := keeper.EligibleWithdrawals(ctx.WithBlockHeight(0))
require.NoError(t, err)
require.Empty(t, withdrawalsByHeight)

// make sure we have no withdrawals below height 1
withdrawalsByHeight, err := keeper.EligibleWithdrawals(ctx, 1, limit)
withdrawalsByHeight, err = keeper.EligibleWithdrawals(ctx.WithBlockHeight(1))
require.NoError(t, err)
require.Empty(t, withdrawalsByHeight)

// make sure we have exactly one withdrawal below height 2
withdrawalsByHeight, err = keeper.EligibleWithdrawals(ctx, 2, limit)
withdrawalsByHeight, err = keeper.EligibleWithdrawals(ctx.WithBlockHeight(2))
require.NoError(t, err)
require.Len(t, withdrawalsByHeight, 1)
matchesTestCase(withdrawalsByHeight[0], inputs[0])

// under height 50 we only have 2 withdrawals
withdrawalsByHeight, err = keeper.EligibleWithdrawals(ctx, 50, limit)
withdrawalsByHeight, err = keeper.EligibleWithdrawals(ctx.WithBlockHeight(50))
require.NoError(t, err)
require.Len(t, withdrawalsByHeight, 2)
matchesTestCase(withdrawalsByHeight[0], inputs[0])
matchesTestCase(withdrawalsByHeight[1], inputs[1])

// under height 1000 we get all of them
withdrawalsByHeight, err = keeper.EligibleWithdrawals(ctx, 1000, limit)
withdrawalsByHeight, err = keeper.EligibleWithdrawals(ctx.WithBlockHeight(1000))
require.NoError(t, err)
require.Len(t, withdrawalsByHeight, 4)
matchesTestCase(withdrawalsByHeight[0], inputs[0])
Expand All @@ -124,9 +127,10 @@ func TestKeeper_withdrawalsPersistence(t *testing.T) {
matchesTestCase(withdrawalsByHeight[3], inputs[3])

// under height 1000 we get the first 2 if we limit the output by 2
withdrawalsByHeight, err = keeper.EligibleWithdrawals(ctx, 1000, limit/2)
keeper.maxWithdrawalsPerBlock /= 2
withdrawalsByHeight, err = keeper.EligibleWithdrawals(ctx.WithBlockHeight(1000))
require.NoError(t, err)
require.Len(t, withdrawalsByHeight, limit/2)
require.Len(t, withdrawalsByHeight, int(keeper.maxWithdrawalsPerBlock))
matchesTestCase(withdrawalsByHeight[0], inputs[0])
matchesTestCase(withdrawalsByHeight[1], inputs[1])
}
Expand Down
47 changes: 25 additions & 22 deletions octane/evmengine/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,19 +25,20 @@ import (
)

type Keeper struct {
cdc codec.BinaryCodec
storeService store.KVStoreService
headTable ExecutionHeadTable
withdrawalTable WithdrawalTable
engineCl ethclient.EngineClient
txConfig client.TxConfig
voteProvider types.VoteExtensionProvider
eventProcs []types.EvmEventProcessor
cmtAPI comet.API
addrProvider types.AddressProvider
feeRecProvider types.FeeRecipientProvider
buildDelay time.Duration
buildOptimistic bool
cdc codec.BinaryCodec
storeService store.KVStoreService
headTable ExecutionHeadTable
withdrawalTable WithdrawalTable
engineCl ethclient.EngineClient
txConfig client.TxConfig
voteProvider types.VoteExtensionProvider
eventProcs []types.EvmEventProcessor
cmtAPI comet.API
addrProvider types.AddressProvider
feeRecProvider types.FeeRecipientProvider
buildDelay time.Duration
buildOptimistic bool
maxWithdrawalsPerBlock uint64

// mutablePayload contains the previous optimistically triggered payload.
// It is optimistic because the validator set can change,
Expand All @@ -57,6 +58,7 @@ func NewKeeper(
txConfig client.TxConfig,
addrProvider types.AddressProvider,
feeRecProvider types.FeeRecipientProvider,
maxWithdrawalsPerBlock uint64,
eventProcs ...types.EvmEventProcessor,
) (*Keeper, error) {
schema := &ormv1alpha1.ModuleSchemaDescriptor{SchemaFile: []*ormv1alpha1.ModuleSchemaDescriptor_FileEntry{
Expand All @@ -78,15 +80,16 @@ func NewKeeper(
}

return &Keeper{
cdc: cdc,
storeService: storeService,
headTable: dbStore.ExecutionHeadTable(),
withdrawalTable: dbStore.WithdrawalTable(),
engineCl: engineCl,
txConfig: txConfig,
addrProvider: addrProvider,
feeRecProvider: feeRecProvider,
eventProcs: eventProcs,
cdc: cdc,
storeService: storeService,
headTable: dbStore.ExecutionHeadTable(),
withdrawalTable: dbStore.WithdrawalTable(),
engineCl: engineCl,
txConfig: txConfig,
addrProvider: addrProvider,
feeRecProvider: feeRecProvider,
eventProcs: eventProcs,
maxWithdrawalsPerBlock: maxWithdrawalsPerBlock,
}, nil
}

Expand Down
3 changes: 2 additions & 1 deletion octane/evmengine/keeper/keeper_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,8 @@ func TestKeeper_isNextProposer(t *testing.T) {
address: nxtAddr,
}
frp := newRandomFeeRecipientProvider()
keeper, err := NewKeeper(cdc, storeService, &mockEngine, txConfig, ap, frp)
maxWithdrawalsPerBlock := uint64(32)
keeper, err := NewKeeper(cdc, storeService, &mockEngine, txConfig, ap, frp, maxWithdrawalsPerBlock)
require.NoError(t, err)
keeper.SetCometAPI(cmtAPI)
populateGenesisHead(ctx, t, keeper)
Expand Down
3 changes: 2 additions & 1 deletion octane/evmengine/keeper/msg_server_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ func Test_msgServer_ExecutionPayload(t *testing.T) {
}
frp := newRandomFeeRecipientProvider()
evmLogProc := mockEventProc{deliverErr: errors.New("test error")}
keeper, err := NewKeeper(cdc, storeService, &mockEngine, txConfig, ap, frp, evmLogProc)
maxWithdrawalsPerBlock := uint64(32)
keeper, err := NewKeeper(cdc, storeService, &mockEngine, txConfig, ap, frp, maxWithdrawalsPerBlock, evmLogProc)
require.NoError(t, err)
keeper.SetCometAPI(cmtAPI)
populateGenesisHead(ctx, t, keeper)
Expand Down
3 changes: 2 additions & 1 deletion octane/evmengine/keeper/proposal_server_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ func Test_proposalServer_ExecutionPayload(t *testing.T) {
sdkCtx = sdkCtx.WithExecMode(sdk.ExecModeFinalize)

frp := newRandomFeeRecipientProvider()
keeper, err := NewKeeper(cdc, storeService, &mockEngine, txConfig, nil, frp)
maxWithdrawalsPerBlock := uint64(32)
keeper, err := NewKeeper(cdc, storeService, &mockEngine, txConfig, nil, frp, maxWithdrawalsPerBlock)
require.NoError(t, err)
populateGenesisHead(sdkCtx, t, keeper)

Expand Down
1 change: 1 addition & 0 deletions octane/evmengine/module/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ func ProvideModule(in ModuleInputs) (ModuleOutputs, error) {
in.TXConfig,
in.AddrProvider,
in.FeeRecProvider,
in.Config.GetMaxWithdrawalsPerBlock(),
eventProcs...,
)
if err != nil {
Expand Down

0 comments on commit 24b6ad1

Please sign in to comment.