Skip to content
This repository has been archived by the owner on Dec 4, 2024. It is now read-only.

Commit

Permalink
Fixed tests and new e2e test for baseFeeChangeDenom network paramet…
Browse files Browse the repository at this point in the history
…er (#1901)

* fixed tests and new e2e test for baseFeeDenom

* comment fix

* fix test `TestGovernanceManager_PostEpoch`

* Fix unit tests

* Fix unit test

* fix governance e2e test

* Minor updates

---------

Co-authored-by: Stefan Negovanović <[email protected]>
  • Loading branch information
dusannosovic-ethernal and Stefan-Ethernal authored Sep 11, 2023
1 parent 33a3010 commit 311afc9
Show file tree
Hide file tree
Showing 13 changed files with 219 additions and 90 deletions.
3 changes: 1 addition & 2 deletions blockchain/blockchain.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (

"github.com/0xPolygon/polygon-edge/blockchain/storage"
"github.com/0xPolygon/polygon-edge/chain"
"github.com/0xPolygon/polygon-edge/forkmanager"
"github.com/0xPolygon/polygon-edge/helper/common"
"github.com/0xPolygon/polygon-edge/state"
"github.com/0xPolygon/polygon-edge/types"
Expand Down Expand Up @@ -1393,7 +1392,7 @@ func (b *Blockchain) CalculateBaseFee(parent *types.Header) uint64 {

func (b *Blockchain) calcBaseFeeDelta(gasUsedDelta, parentGasTarget, baseFee uint64) uint64 {
baseFeeChangeDenom := chain.BaseFeeChangeDenom
if forkmanager.GetInstance().IsForkEnabled(chain.Governance, b.Header().Number) {
if b.config.Params.Forks.IsActive(chain.Governance, b.Header().Number) {
baseFeeChangeDenom = b.Config().BaseFeeChangeDenom
}

Expand Down
7 changes: 7 additions & 0 deletions blockchain/blockchain_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1403,6 +1403,13 @@ func TestBlockchain_CalculateBaseFee(t *testing.T) {
},
}

blockchain.setCurrentHeader(&types.Header{
Number: test.blockNumber + 1,
GasLimit: test.parentGasLimit,
GasUsed: test.parentGasUsed,
BaseFee: test.parentBaseFee,
}, big.NewInt(1))

parent := &types.Header{
Number: test.blockNumber,
GasLimit: test.parentGasLimit,
Expand Down
4 changes: 0 additions & 4 deletions consensus/polybft/common/polybft_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,6 @@ type PolyBFTConfig struct {
// WithdrawalWaitPeriod indicates a number of epochs after which withdrawal can be done from child chain
WithdrawalWaitPeriod uint64 `json:"withdrawalWaitPeriod,omitempty"`

// TODO: @Stefan-Ethernal REMOVE!
// BaseFeeChangeDenom is the value to bound the amount the base fee can change between blocks
BaseFeeChangeDenom uint64 `json:"baseFeeChangeDenom,omitempty"`

// RewardConfig defines rewards configuration
RewardConfig *RewardsConfig `json:"rewardConfig"`

Expand Down
20 changes: 12 additions & 8 deletions consensus/polybft/consensus_runtime_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -215,10 +215,10 @@ func TestConsensusRuntime_OnBlockInserted_EndOfEpoch(t *testing.T) {
txPool.On("ResetWithHeaders", mock.Anything).Once()

snapshot := NewProposerSnapshot(epochSize-1, validatorSet)
polybftCfg := &polyCommon.PolyBFTConfig{EpochSize: epochSize}
config := &runtimeConfig{
GenesisConfig: &polyCommon.PolyBFTConfig{
EpochSize: epochSize,
},
GenesisConfig: polybftCfg,
genesisParams: &chain.Params{Engine: map[string]interface{}{polyCommon.ConsensusName: polybftCfg}},
blockchain: blockchainMock,
polybftBackend: polybftBackendMock,
txPool: txPool,
Expand All @@ -238,11 +238,14 @@ func TestConsensusRuntime_OnBlockInserted_EndOfEpoch(t *testing.T) {
FirstBlockInEpoch: header.Number - epochSize + 1,
CurrentClientConfig: config.GenesisConfig,
},
lastBuiltBlock: &types.Header{Number: header.Number - 1},
stateSyncManager: &dummyStateSyncManager{},
checkpointManager: &dummyCheckpointManager{},
stakeManager: &dummyStakeManager{},
governanceManager: &dummyGovernanceManager{},
lastBuiltBlock: &types.Header{Number: header.Number - 1},
stateSyncManager: &dummyStateSyncManager{},
checkpointManager: &dummyCheckpointManager{},
stakeManager: &dummyStakeManager{},
governanceManager: &dummyGovernanceManager{
getClientConfigFn: func() (*chain.Params, error) {
return config.genesisParams, nil
}},
doubleSigningTracker: tracker,
}
runtime.OnBlockInserted(&types.FullBlock{Block: builtBlock})
Expand Down Expand Up @@ -490,6 +493,7 @@ func Test_NewConsensusRuntime(t *testing.T) {
config := &runtimeConfig{
polybftBackend: polybftBackendMock,
State: newTestState(t),
genesisParams: &chain.Params{Engine: map[string]interface{}{polyCommon.ConsensusName: polyBftConfig}},
GenesisConfig: polyBftConfig,
DataDir: tmpDir,
Key: createTestKey(t),
Expand Down
5 changes: 3 additions & 2 deletions consensus/polybft/contracts_initializer.go
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,8 @@ func isNativeRewardToken(cfg common.PolyBFTConfig) bool {
}

// initNetworkParamsContract initializes NetworkParams contract on child chain
func initNetworkParamsContract(cfg common.PolyBFTConfig, transition *state.Transition) error {
func initNetworkParamsContract(baseFeeChangeDenom uint64, cfg common.PolyBFTConfig,
transition *state.Transition) error {
initFn := &contractsapi.InitializeNetworkParamsFn{
InitParams: &contractsapi.InitParams{
// only timelock controller can execute transactions on network params
Expand All @@ -293,12 +294,12 @@ func initNetworkParamsContract(cfg common.PolyBFTConfig, transition *state.Trans
NewMinValidatorSetSize: new(big.Int).SetUint64(cfg.MinValidatorSetSize),
NewMaxValidatorSetSize: new(big.Int).SetUint64(cfg.MaxValidatorSetSize),
NewWithdrawalWaitPeriod: new(big.Int).SetUint64(cfg.WithdrawalWaitPeriod),
NewBaseFeeChangeDenom: new(big.Int).SetUint64(cfg.BaseFeeChangeDenom),
NewBlockTime: new(big.Int).SetUint64(uint64(cfg.BlockTime.Duration)),
NewBlockTimeDrift: new(big.Int).SetUint64(cfg.BlockTimeDrift),
NewVotingDelay: new(big.Int).Set(cfg.GovernanceConfig.VotingDelay),
NewVotingPeriod: new(big.Int).Set(cfg.GovernanceConfig.VotingPeriod),
NewProposalThreshold: new(big.Int).Set(cfg.GovernanceConfig.ProposalThreshold),
NewBaseFeeChangeDenom: new(big.Int).SetUint64(baseFeeChangeDenom),
},
}

Expand Down
1 change: 1 addition & 0 deletions consensus/polybft/contractsapi/bindings-gen/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -419,6 +419,7 @@ func main() {
"initialize",
"setNewEpochSize",
"setNewSprintSize",
"setNewBaseFeeChangeDenom",
},
[]string{
"NewCheckpointBlockInterval",
Expand Down
16 changes: 16 additions & 0 deletions consensus/polybft/contractsapi/contractsapi.go

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

12 changes: 11 additions & 1 deletion consensus/polybft/governance_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,17 @@ var _ GovernanceManager = (*dummyGovernanceManager)(nil)

// dummyStakeManager is a dummy implementation of GovernanceManager interface
// used only for unit testing
type dummyGovernanceManager struct{}
type dummyGovernanceManager struct {
getClientConfigFn func() (*chain.Params, error)
}

func (d *dummyGovernanceManager) PostBlock(req *polyCommon.PostBlockRequest) error { return nil }
func (d *dummyGovernanceManager) PostEpoch(req *polyCommon.PostEpochRequest) error { return nil }
func (d *dummyGovernanceManager) GetClientConfig() (*chain.Params, error) {
if d.getClientConfigFn != nil {
return d.getClientConfigFn()
}

return nil, nil
}

Expand Down Expand Up @@ -369,6 +375,10 @@ func (g *governanceManager) PostEpoch(req *polyCommon.PostEpochRequest) error {
}
}

if len(eventsRaw) > 0 {
latestChainParams.Engine[polyCommon.ConsensusName] = latestPolybftConfig
}

// save updated config to db
return g.state.GovernanceStore.insertClientConfig(latestChainParams)
}
Expand Down
93 changes: 51 additions & 42 deletions consensus/polybft/governance_manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,44 +15,55 @@ import (
"github.com/umbracle/ethgo/abi"
)

// TODO: @Stefan-Ethernal FIXME
// func TestGovernanceManager_PostEpoch(t *testing.T) {
// t.Parallel()

// state := newTestState(t)
// governanceManager := &governanceManager{
// state: state,
// logger: hclog.NewNullLogger(),
// }

// // insert some governance event
// epochRewardEvent := &contractsapi.NewEpochRewardEvent{Reward: big.NewInt(10_000)}
// require.NoError(t, state.GovernanceStore.insertGovernanceEvents(1, 7, []contractsapi.EventAbi{epochRewardEvent}))
// // insert last processed block
// require.NoError(t, state.GovernanceStore.insertLastProcessed(20))

// // no initial config was saved, so we expect an error
// require.ErrorIs(t, governanceManager.PostEpoch(&common.PostEpochRequest{
// NewEpochID: 2,
// FirstBlockOfEpoch: 21,
// Forks: &chain.Forks{chain.Governance: chain.NewFork(0)},
// }),
// errClientConfigNotFound)

// // insert initial config
// require.NoError(t, state.GovernanceStore.insertClientConfig(createTestPolybftConfig()))

// // PostEpoch will now update config with new epoch reward value
// require.NoError(t, governanceManager.PostEpoch(&common.PostEpochRequest{
// NewEpochID: 2,
// FirstBlockOfEpoch: 21,
// Forks: &chain.Forks{chain.Governance: chain.NewFork(0)},
// }))

// updatedConfig, err := state.GovernanceStore.getClientConfig()
// require.NoError(t, err)
// require.Equal(t, epochRewardEvent.Reward.Uint64(), updatedConfig.EpochReward)
// }
func TestGovernanceManager_PostEpoch(t *testing.T) {
t.Parallel()

state := newTestState(t)
governanceManager := &governanceManager{
state: state,
logger: hclog.NewNullLogger(),
}

// insert some governance event
baseFeeChangeDenomEvent := &contractsapi.NewBaseFeeChangeDenomEvent{BaseFeeChangeDenom: big.NewInt(100)}
epochRewardEvent := &contractsapi.NewEpochRewardEvent{Reward: big.NewInt(10000)}

require.NoError(t, state.GovernanceStore.insertGovernanceEvents(1, 7, []contractsapi.EventAbi{baseFeeChangeDenomEvent, epochRewardEvent}))
// insert last processed block
require.NoError(t, state.GovernanceStore.insertLastProcessed(20))

// no initial config was saved, so we expect an error
require.ErrorIs(t, governanceManager.PostEpoch(&common.PostEpochRequest{
NewEpochID: 2,
FirstBlockOfEpoch: 21,
Forks: &chain.Forks{chain.Governance: chain.NewFork(0)},
}),
errClientConfigNotFound)

params := &chain.Params{
BaseFeeChangeDenom: 8,
Engine: map[string]interface{}{common.ConsensusName: createTestPolybftConfig()},
}

// insert initial config
require.NoError(t, state.GovernanceStore.insertClientConfig(params))

// PostEpoch will now update config with new epoch reward value
require.NoError(t, governanceManager.PostEpoch(&common.PostEpochRequest{
NewEpochID: 2,
FirstBlockOfEpoch: 21,
Forks: &chain.Forks{chain.Governance: chain.NewFork(0)},
}))

updatedConfig, err := state.GovernanceStore.getClientConfig()
require.NoError(t, err)
require.Equal(t, baseFeeChangeDenomEvent.BaseFeeChangeDenom.Uint64(), updatedConfig.BaseFeeChangeDenom)

pbftConfig, err := common.GetPolyBFTConfig(updatedConfig)
require.NoError(t, err)

require.Equal(t, epochRewardEvent.Reward.Uint64(), pbftConfig.EpochReward)
}

func TestGovernanceManager_PostBlock(t *testing.T) {
t.Parallel()
Expand All @@ -79,8 +90,7 @@ func TestGovernanceManager_PostBlock(t *testing.T) {
Number: 0,
})

chainParams := &chain.Params{}
chainParams.Engine[common.ConsensusName] = genesisPolybftConfig
chainParams := &chain.Params{Engine: map[string]interface{}{common.ConsensusName: genesisPolybftConfig}}
governanceManager, err := newGovernanceManager(chainParams, genesisPolybftConfig,
hclog.NewNullLogger(), state, blockchainMock)
require.NoError(t, err)
Expand Down Expand Up @@ -126,8 +136,7 @@ func TestGovernanceManager_PostBlock(t *testing.T) {
Number: 4,
})

chainParams := &chain.Params{}
chainParams.Engine[common.ConsensusName] = genesisPolybftConfig
chainParams := &chain.Params{Engine: map[string]interface{}{common.ConsensusName: genesisPolybftConfig}}
governanceManager, err := newGovernanceManager(chainParams, genesisPolybftConfig,
hclog.NewNullLogger(), state, blockchainMock)
require.NoError(t, err)
Expand Down
2 changes: 1 addition & 1 deletion consensus/polybft/polybft.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ func GenesisPostHookFactory(config *chain.Chain, engineName string) func(txn *st
}

// initialize NetworkParams SC
if err = initNetworkParamsContract(polyBFTConfig, transition); err != nil {
if err = initNetworkParamsContract(config.Params.BaseFeeChangeDenom, polyBFTConfig, transition); err != nil {
return err
}

Expand Down
5 changes: 3 additions & 2 deletions consensus/polybft/sc_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,8 @@ func TestIntegratoin_PerformExit(t *testing.T) {
func TestIntegration_CommitEpoch(t *testing.T) {
t.Parallel()

baseFeeChangeDenom := uint64(25)

// init validator sets
validatorSetSize := []int{5, 10, 50, 100}

Expand Down Expand Up @@ -335,7 +337,6 @@ func TestIntegration_CommitEpoch(t *testing.T) {
MaxValidatorSetSize: 100,
CheckpointInterval: 900,
WithdrawalWaitPeriod: 1,
BaseFeeChangeDenom: 20,
BlockTimeDrift: 10,
// use 1st account as governance address
Governance: currentValidators.ToValidatorSet().Accounts().GetAddresses()[0],
Expand Down Expand Up @@ -367,7 +368,7 @@ func TestIntegration_CommitEpoch(t *testing.T) {
require.NoError(t, err)

// init NetworkParams
err = initNetworkParamsContract(polyBFTConfig, transition)
err = initNetworkParamsContract(baseFeeChangeDenom, polyBFTConfig, transition)
require.NoError(t, err)

// create input for commit epoch
Expand Down
Loading

0 comments on commit 311afc9

Please sign in to comment.