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

Revisit proposer config management #6325

Merged
merged 9 commits into from
Oct 31, 2022

Conversation

tbenr
Copy link
Contributor

@tbenr tbenr commented Oct 14, 2022

Summary

  • moves proposer config management away from BeaconProposerPreparer, introducing ProposerConfigManager
  • ProposerConfigManager is now responsible for:
    • refreshing the config when requested.
    • resolving attribute values applying the layered fallbacks in this sequence:
      1. proposerConfig specific config
      2. runtimeConfig specific config
      3. proposerConfig default config
      4. default values specified via CLI
    • exposing ProposerConfigPropertiesProvider (previously named ValidatorRegistrationPropertiesProvider)
    • providing validator ownership check via isOwnedValidator used by KeyManager API to handle 404 more explicitly
  • removes logic from ProposerConfig (unit tests removed too) everything is now in ProposerConfigManager

UX change

From UX perspective the notable change (breaking change) is that attributes specified in the default_config are now applied to all validators, including the one specified in the proposer_config. This means that in proposer_config you can specify only the attributes you want to override (or add) from the default_config.
The previous behaviour was that default_config configuration were considered only for validator not explicitly configured under proposer_config.

So now fee_recipient and builder.enabled are only mandatory in the default config, all the other attributes can be optionally specified.

example:

This kind of configurations are now possible:

{
  "default_config": {
    "fee_recipient": "0x6e35733c5af9B61374A128e6F85f553aF09ff89A"
    "builder": {
      "enabled": false
      "gas_limit": "12345654321"
    }
  },
  "proposer_config": {
    "0xa0578...": {
      "builder": {
        "enabled": true,
      }
    }
  }
}

the resulting config for 0xa0578... will be:

"0xa0578...": {
   "fee_recipient": "0x6e35733c5af9B61374A128e6F85f553aF09ff89A"
    "builder": {
      "enabled": true,
      "gas_limit": "12345654321"
    }
 }

fixes #6314

Suggested documentation changes under validators-proposer-config section:

  • default_config - (required) A default proposer configuration which contains all default values that will be applied to every validator. These values can be overridden for specific validators in proposer_config.
  • proposer_config - (optional) A proposer configuration for multiple validator public keys.

Proposer configuration attributes:

fee_recipient (optional in proposal_config but is mandatory for default_config)
fee recipient to be used when proposing blocks.

builder (optional). includes three attributes:

  • enabled - (optional in proposal_config but is mandatory for default_config) specifies whether to use the builder endpoint when proposing blocks.
  • gas_limit - (optional) specifies the gas_limit for the builder. The default is 30000000.
  • registration_overrides (optional) specifies dedicated overrides to be used during the registration process. Useful for DVT and SSV technologies
    • timestamp (optional) timestamp to be used (instead of current time) in validator registration message
    • public_key (optional in proposal_config but is forbidden for default_config) public key to be used (instead of validator's public key) in validator registration message

!!! example "proposerConfig.json"

    {
      "proposer_config": {
        "0xa057816155ad77931185101128655c0191bd0214c201ca48ed887f6c4c6adf334070efcd75140eada5ac83a92506dd7a": {
          "fee_recipient": "0x50155530FCE8a85ec7055A5F8b2bE214B3DaeFd3",
          "builder": {
            "enabled": true,
            "gas_limit": "35000000"
          }
        },
"0xa99a76ed7796f7be22d5b7e85deeb7c5677e88e511e0b337618f8c4eb61349b4bf2d153f649f7b53359fe8b94a38e44c": {
          "builder": {
            "enabled": true
          }
        },
      },
      "default_config": {
        "fee_recipient": "0x6e35733c5af9B61374A128e6F85f553aF09ff89A"
        "builder": {
          "enabled": false,
          "gas_limit": "25000000"
        }
      }
    }

In the example, validator 0xa0578... will be configured as:

"fee_recipient": "0x50155530FCE8a85ec7055A5F8b2bE214B3DaeFd3",
"builder": {
  "enabled": true,
  "gas_limit": "35000000"
}

validator 0xa99a7... will be configured as:

"fee_recipient": "0x6e35733c5af9B61374A128e6F85f553aF09ff89A",
"builder": {
  "enabled": true,
  "gas_limit": "25000000"
}

all other validators will will be configured as:

"fee_recipient": "0x6e35733c5af9B61374A128e6F85f553aF09ff89A"
"builder": {
  "enabled": false
}

Documentation

  • I thought about documentation and added the doc-change-required label to this PR if updates are required.

Changelog

  • I thought about adding a changelog entry, and added one if I deemed necessary.

@tbenr tbenr added the doc-change-required Indicates an issue or PR that requires doc to be updated label Oct 14, 2022
@tbenr tbenr force-pushed the overlaying_proposerConfig_settings branch from a166219 to fc4d08e Compare October 26, 2022 13:56
@tbenr tbenr marked this pull request as ready for review October 26, 2022 17:59
@tbenr
Copy link
Contributor Author

tbenr commented Oct 27, 2022

@ciaranmcveigh5 I've worked on this change and I'd like to have a check with you WRT of registration_overrides.

Giving this example:

{
  "proposer_config": {
    "0xa05...": {
      "fee_recipient": "0x501...",
      "builder": {
        "enabled": true
      }
    }
  },
  "default_config": {
    "fee_recipient": "0x6e35...",
    "builder": {
      "enabled": true,
        "registration_overrides": {
          "timestamp": "1234",
          "public_key": "0xb53d21a4cfd...."
        }
    }
  }
}

The current behaviour is that override will NOT be applied to 0xa05... but it will be applied to all the other validators.

With this PR the override WILL be applied to 0xa05... too.
So as long as you specify the override on the default config, all validators will get it. The only think you can do per validator is changing the values, you can turn it off anymore.

My question is: how are you using this configuration in real world scenario?
I'm incline to prevent usage of public_key in default_config if it is inline with your usecases

Copy link
Contributor

@rolfyone rolfyone left a comment

Choose a reason for hiding this comment

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

LGTM overall, we definitely want a changelog so that people are aware this behaviour is changing... Im not sure if anyone is relying on the current quirks...

@ciaranmcveigh5
Copy link
Contributor

ciaranmcveigh5 commented Oct 31, 2022

Hey @tbenr

Agree with this

"I'm inclined to prevent usage of public_key in default_config if it is inline with your usecases"

The use case is for distributed validators so the public key section should always be specific to another public key rather than default which would apply to all validators

I think this was an interim fix for a single validator while we were working out the best way to implement it.

These changes shouldn't impact our proposer-config setup.

Thanks for checking,

cc @corverroos @OisinKyne

@tbenr tbenr force-pushed the overlaying_proposerConfig_settings branch from 188862e to 574430c Compare October 31, 2022 14:31
@tbenr tbenr merged commit ec55d97 into Consensys:master Oct 31, 2022
@tbenr tbenr deleted the overlaying_proposerConfig_settings branch October 31, 2022 16:21
@alexandratran alexandratran removed the doc-change-required Indicates an issue or PR that requires doc to be updated label Nov 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Apply settings from default_config in the proposerConfig file more intuitively
4 participants