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

Insert stored fee recipient for ForkchoiceUpdated call #10349

Merged
merged 9 commits into from
Mar 14, 2022
4 changes: 2 additions & 2 deletions beacon-chain/db/iface/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ type ReadOnlyDatabase interface {
// Powchain operations.
PowchainData(ctx context.Context) (*ethpb.ETH1ChainData, error)
// Fee reicipients operations.
FeeRecipientByValidatorID(ctx context.Context, id uint64) (common.Address, error)
FeeRecipientByValidatorID(ctx context.Context, id types.ValidatorIndex) (common.Address, error)
// origin checkpoint sync support
OriginBlockRoot(ctx context.Context) ([32]byte, error)
}
Expand Down Expand Up @@ -81,7 +81,7 @@ type NoHeadAccessDatabase interface {
// Run any required database migrations.
RunMigrations(ctx context.Context) error
// Fee reicipients operations.
SaveFeeRecipientsByValidatorIDs(ctx context.Context, ids []uint64, addrs []common.Address) error
SaveFeeRecipientsByValidatorIDs(ctx context.Context, ids []types.ValidatorIndex, addrs []common.Address) error

CleanUpDirtyStates(ctx context.Context, slotsPerArchivedPoint types.Slot) error
}
Expand Down
8 changes: 4 additions & 4 deletions beacon-chain/db/kv/blocks.go
Original file line number Diff line number Diff line change
Expand Up @@ -395,13 +395,13 @@ func (s *Store) HighestSlotBlocksBelow(ctx context.Context, slot types.Slot) ([]

// FeeRecipientByValidatorID returns the fee recipient for a validator id.
// `ErrNotFoundFeeRecipient` is returned if the validator id is not found.
func (s *Store) FeeRecipientByValidatorID(ctx context.Context, id uint64) (common.Address, error) {
func (s *Store) FeeRecipientByValidatorID(ctx context.Context, id types.ValidatorIndex) (common.Address, error) {
ctx, span := trace.StartSpan(ctx, "BeaconDB.FeeRecipientByValidatorID")
defer span.End()
var addr []byte
err := s.db.View(func(tx *bolt.Tx) error {
bkt := tx.Bucket(feeRecipientBucket)
addr = bkt.Get(bytesutil.Uint64ToBytesBigEndian(id))
addr = bkt.Get(bytesutil.Uint64ToBytesBigEndian(uint64(id)))
if addr == nil {
return errors.Wrapf(ErrNotFoundFeeRecipient, "validator id %d", id)
}
Expand All @@ -412,7 +412,7 @@ func (s *Store) FeeRecipientByValidatorID(ctx context.Context, id uint64) (commo

// SaveFeeRecipientsByValidatorIDs saves the fee recipients for validator ids.
// Error is returned if `ids` and `recipients` are not the same length.
func (s *Store) SaveFeeRecipientsByValidatorIDs(ctx context.Context, ids []uint64, feeRecipients []common.Address) error {
func (s *Store) SaveFeeRecipientsByValidatorIDs(ctx context.Context, ids []types.ValidatorIndex, feeRecipients []common.Address) error {
_, span := trace.StartSpan(ctx, "BeaconDB.SaveFeeRecipientByValidatorID")
defer span.End()

Expand All @@ -423,7 +423,7 @@ func (s *Store) SaveFeeRecipientsByValidatorIDs(ctx context.Context, ids []uint6
return s.db.Update(func(tx *bolt.Tx) error {
bkt := tx.Bucket(feeRecipientBucket)
for i, id := range ids {
if err := bkt.Put(bytesutil.Uint64ToBytesBigEndian(id), feeRecipients[i].Bytes()); err != nil {
if err := bkt.Put(bytesutil.Uint64ToBytesBigEndian(uint64(id)), feeRecipients[i].Bytes()); err != nil {
return err
}
}
Expand Down
4 changes: 2 additions & 2 deletions beacon-chain/db/kv/blocks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -595,11 +595,11 @@ func TestStore_BlocksBySlot_BlockRootsBySlot(t *testing.T) {
func TestStore_FeeRecipientByValidatorID(t *testing.T) {
db := setupDB(t)
ctx := context.Background()
ids := []uint64{0, 0, 0}
ids := []types.ValidatorIndex{0, 0, 0}
feeRecipients := []common.Address{{}, {}, {}, {}}
require.ErrorContains(t, "validatorIDs and feeRecipients must be the same length", db.SaveFeeRecipientsByValidatorIDs(ctx, ids, feeRecipients))

ids = []uint64{0, 1, 2}
ids = []types.ValidatorIndex{0, 1, 2}
feeRecipients = []common.Address{{'a'}, {'b'}, {'c'}}
require.NoError(t, db.SaveFeeRecipientsByValidatorIDs(ctx, ids, feeRecipients))
f, err := db.FeeRecipientByValidatorID(ctx, 0)
Expand Down
19 changes: 14 additions & 5 deletions beacon-chain/node/config.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package node

import (
"fmt"

"github.com/ethereum/go-ethereum/common"
types "github.com/prysmaticlabs/eth2-types"
"github.com/prysmaticlabs/prysm/cmd"
Expand Down Expand Up @@ -105,10 +107,17 @@ func configureInteropConfig(cliCtx *cli.Context) {
}
}

func configureExecutionSetting(cliCtx *cli.Context) {
if cliCtx.IsSet(flags.FeeRecipient.Name) {
c := params.BeaconConfig()
c.FeeRecipient = common.HexToAddress(cliCtx.String(flags.FeeRecipient.Name))
params.OverrideBeaconConfig(c)
func configureExecutionSetting(cliCtx *cli.Context) error {
if !cliCtx.IsSet(flags.FeeRecipient.Name) {
return nil
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe we can add a log here to inform the user that the validator will be sending the address or else it defaults to burn address.

}

c := params.BeaconConfig()
ha := cliCtx.String(flags.FeeRecipient.Name)
if !common.IsHexAddress(ha) {
return fmt.Errorf("%s is not a valid fee recipient address", ha)
}
c.DefaultFeeRecipient = common.HexToAddress(ha)
params.OverrideBeaconConfig(c)
return nil
}
17 changes: 14 additions & 3 deletions beacon-chain/node/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,20 @@ func TestConfigureExecutionSetting(t *testing.T) {
set.String(flags.FeeRecipient.Name, "", "")
require.NoError(t, set.Set(flags.FeeRecipient.Name, "0xB"))
cliCtx := cli.NewContext(&app, set, nil)

configureExecutionSetting(cliCtx)
assert.Equal(t, common.HexToAddress("0xB"), params.BeaconConfig().FeeRecipient)
err := configureExecutionSetting(cliCtx)
require.ErrorContains(t, "0xB is not a valid fee recipient address", err)

require.NoError(t, set.Set(flags.FeeRecipient.Name, "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"))
cliCtx = cli.NewContext(&app, set, nil)
err = configureExecutionSetting(cliCtx)
require.NoError(t, err)
assert.Equal(t, common.HexToAddress("AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"), params.BeaconConfig().DefaultFeeRecipient)

require.NoError(t, set.Set(flags.FeeRecipient.Name, "0xAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"))
cliCtx = cli.NewContext(&app, set, nil)
err = configureExecutionSetting(cliCtx)
require.NoError(t, err)
assert.Equal(t, common.HexToAddress("0xAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"), params.BeaconConfig().DefaultFeeRecipient)
}

func TestConfigureNetwork(t *testing.T) {
Expand Down
4 changes: 3 additions & 1 deletion beacon-chain/node/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,9 @@ func New(cliCtx *cli.Context, opts ...Option) (*BeaconNode, error) {
configureEth1Config(cliCtx)
configureNetwork(cliCtx)
configureInteropConfig(cliCtx)
configureExecutionSetting(cliCtx)
if err := configureExecutionSetting(cliCtx); err != nil {
return nil, err
}

// Initializes any forks here.
params.BeaconConfig().InitializeForkSchedule()
Expand Down
6 changes: 3 additions & 3 deletions beacon-chain/rpc/eth/beacon/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ func TestGetSpec(t *testing.T) {
config.TerminalBlockHash = common.HexToHash("TerminalBlockHash")
config.TerminalBlockHashActivationEpoch = 72
config.TerminalTotalDifficulty = "73"
config.FeeRecipient = common.HexToAddress("FeeRecipient")
config.DefaultFeeRecipient = common.HexToAddress("DefaultFeeRecipient")

var dbp [4]byte
copy(dbp[:], []byte{'0', '0', '0', '1'})
Expand Down Expand Up @@ -329,8 +329,8 @@ func TestGetSpec(t *testing.T) {
assert.Equal(t, common.HexToHash("TerminalBlockHash"), common.HexToHash(v))
case "TERMINAL_TOTAL_DIFFICULTY":
assert.Equal(t, "73", v)
case "FeeRecipient":
assert.Equal(t, common.HexToAddress("FeeRecipient"), v)
case "DefaultFeeRecipient":
assert.Equal(t, common.HexToAddress("DefaultFeeRecipient"), v)
case "PROPORTIONAL_SLASHING_MULTIPLIER_BELLATRIX":
assert.Equal(t, "3", v)
case "MIN_SLASHING_PENALTY_QUOTIENT_BELLATRIX":
Expand Down
1 change: 1 addition & 0 deletions beacon-chain/rpc/prysm/v1alpha1/validator/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ go_library(
"//beacon-chain/core/transition/interop:go_default_library",
"//beacon-chain/core/validators:go_default_library",
"//beacon-chain/db:go_default_library",
"//beacon-chain/db/kv:go_default_library",
"//beacon-chain/operations/attestations:go_default_library",
"//beacon-chain/operations/slashings:go_default_library",
"//beacon-chain/operations/synccommittee:go_default_library",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ func (vs *Server) getBellatrixBeaconBlock(ctx context.Context, req *ethpb.BlockR
return nil, err
}

payload, err := vs.getExecutionPayload(ctx, req.Slot)
payload, err := vs.getExecutionPayload(ctx, req.Slot, altairBlk.ProposerIndex)
if err != nil {
return nil, err
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"github.com/prysmaticlabs/prysm/beacon-chain/core/helpers"
"github.com/prysmaticlabs/prysm/beacon-chain/core/time"
"github.com/prysmaticlabs/prysm/beacon-chain/core/transition"
"github.com/prysmaticlabs/prysm/beacon-chain/db/kv"
fieldparams "github.com/prysmaticlabs/prysm/config/fieldparams"
"github.com/prysmaticlabs/prysm/config/params"
"github.com/prysmaticlabs/prysm/encoding/bytesutil"
Expand All @@ -24,7 +25,7 @@ import (

// This returns the execution payload of a given slot. The function has full awareness of pre and post merge.
// The payload is computed given the respected time of merge.
func (vs *Server) getExecutionPayload(ctx context.Context, slot types.Slot) (*enginev1.ExecutionPayload, error) {
func (vs *Server) getExecutionPayload(ctx context.Context, slot types.Slot, vIdx types.ValidatorIndex) (*enginev1.ExecutionPayload, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

this makes sense now

st, err := vs.HeadFetcher.HeadState(ctx)
if err != nil {
return nil, err
Expand Down Expand Up @@ -94,10 +95,24 @@ func (vs *Server) getExecutionPayload(ctx context.Context, slot types.Slot) (*en
SafeBlockHash: parentHash,
FinalizedBlockHash: finalizedBlockHash,
}

feeRecipient := params.BeaconConfig().DefaultFeeRecipient
recipient, err := vs.BeaconDB.FeeRecipientByValidatorID(ctx, vIdx)
switch err == nil {
case true:
feeRecipient = recipient
case errors.As(err, kv.ErrNotFoundFeeRecipient): // If fee recipient is not found, use the default fee recipient.
logrus.WithError(err).WithFields(logrus.Fields{
"validatorIndex": vIdx,
"defaultFeeRecipient": feeRecipient,
}).Error("Fee recipient not found. Using default fee recipient.")
default:
return nil, errors.Wrap(err, "could not get fee recipient in db")
}
p := &enginev1.PayloadAttributes{
Timestamp: uint64(t.Unix()),
PrevRandao: random,
SuggestedFeeRecipient: params.BeaconConfig().FeeRecipient.Bytes(),
SuggestedFeeRecipient: feeRecipient.Bytes(),
}
payloadID, _, err := vs.ExecutionEngineCaller.ForkchoiceUpdated(ctx, f, p)
if err != nil {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ func TestServer_getExecutionPayload(t *testing.T) {
beaconDB := dbTest.SetupDB(t)
require.NoError(t, beaconDB.SaveBlock(context.Background(), b1))
require.NoError(t, beaconDB.SaveBlock(context.Background(), b2))
require.NoError(t, beaconDB.SaveFeeRecipientsByValidatorIDs(context.Background(), []types.ValidatorIndex{0}, []common.Address{{}}))

tests := []struct {
name string
Expand All @@ -88,17 +89,24 @@ func TestServer_getExecutionPayload(t *testing.T) {
payloadID *pb.PayloadIDBytes
terminalBlockHash common.Hash
activationEpoch types.Epoch
validatorIndx types.ValidatorIndex
}{
{
name: "transition completed, nil payload id",
st: transitionSt,
errString: "nil payload id",
},
{
name: "transition completed, happy case",
name: "transition completed, happy case (has fee recipient in Db)",
st: transitionSt,
payloadID: &pb.PayloadIDBytes{0x1},
},
{
name: "transition completed, happy case (doesn't have fee recipient in Db)",
st: transitionSt,
payloadID: &pb.PayloadIDBytes{0x1},
validatorIndx: 1,
},
{
name: "transition completed, could not prepare payload",
st: transitionSt,
Expand Down Expand Up @@ -129,7 +137,7 @@ func TestServer_getExecutionPayload(t *testing.T) {
HeadFetcher: &chainMock.ChainService{State: tt.st},
BeaconDB: beaconDB,
}
_, err := vs.getExecutionPayload(context.Background(), tt.st.Slot())
_, err := vs.getExecutionPayload(context.Background(), tt.st.Slot(), tt.validatorIndx)
if tt.errString != "" {
require.ErrorContains(t, tt.errString, err)
} else {
Expand Down
8 changes: 8 additions & 0 deletions beacon-chain/rpc/prysm/v1alpha1/validator/proposer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import (
"github.com/prysmaticlabs/prysm/testing/require"
"github.com/prysmaticlabs/prysm/testing/util"
"github.com/prysmaticlabs/prysm/time/slots"
logTest "github.com/sirupsen/logrus/hooks/test"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
"google.golang.org/protobuf/proto"
Expand Down Expand Up @@ -2233,6 +2234,7 @@ func TestProposer_GetBeaconBlock_PostForkEpoch(t *testing.T) {
func TestProposer_GetBeaconBlock_BellatrixEpoch(t *testing.T) {
db := dbutil.SetupDB(t)
ctx := context.Background()
hook := logTest.NewGlobal()

terminalBlockHash := bytesutil.PadTo([]byte{0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF}, 32)
Expand Down Expand Up @@ -2333,6 +2335,7 @@ func TestProposer_GetBeaconBlock_BellatrixEpoch(t *testing.T) {
PayloadIDBytes: &enginev1.PayloadIDBytes{1},
ExecutionPayload: payload,
},
BeaconDB: db,
}

randaoReveal, err := util.RandaoReveal(beaconState, 0, privKeys)
Expand All @@ -2346,6 +2349,10 @@ func TestProposer_GetBeaconBlock_BellatrixEpoch(t *testing.T) {
Graffiti: graffiti[:],
}

proposerIndex := types.ValidatorIndex(40)
addr := common.Address{'a'}
require.NoError(t, proposerServer.BeaconDB.SaveFeeRecipientsByValidatorIDs(ctx, []types.ValidatorIndex{proposerIndex}, []common.Address{addr}))

block, err := proposerServer.GetBeaconBlock(ctx, req)
require.NoError(t, err)
bellatrixBlk, ok := block.GetBlock().(*ethpb.GenericBeaconBlock_Bellatrix)
Expand All @@ -2356,6 +2363,7 @@ func TestProposer_GetBeaconBlock_BellatrixEpoch(t *testing.T) {
assert.DeepEqual(t, randaoReveal, bellatrixBlk.Bellatrix.Body.RandaoReveal, "Expected block to have correct randao reveal")
assert.DeepEqual(t, req.Graffiti, bellatrixBlk.Bellatrix.Body.Graffiti, "Expected block to have correct Graffiti")

require.LogsDoNotContain(t, hook, "Fee recipient not found. Using default fee recipient.")
require.DeepEqual(t, payload, bellatrixBlk.Bellatrix.Body.ExecutionPayload) // Payload should equal.
}

Expand Down
2 changes: 1 addition & 1 deletion config/params/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ type BeaconChainConfig struct {
TerminalBlockHash common.Hash `yaml:"TERMINAL_BLOCK_HASH" spec:"true"` // TerminalBlockHash of beacon chain.
TerminalBlockHashActivationEpoch types.Epoch `yaml:"TERMINAL_BLOCK_HASH_ACTIVATION_EPOCH" spec:"true"` // TerminalBlockHashActivationEpoch of beacon chain.
TerminalTotalDifficulty string `yaml:"TERMINAL_TOTAL_DIFFICULTY" spec:"true"` // TerminalTotalDifficulty is part of the experimental Bellatrix spec. This value is type is currently TBD.
FeeRecipient common.Address // FeeRecipient where the transaction fee goes to.
DefaultFeeRecipient common.Address // DefaultFeeRecipient where the transaction fee goes to.
}

// InitializeForkSchedule initializes the schedules forks baked into the config.
Expand Down