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

[Merged by Bors] - Register validator api #3194

Closed
wants to merge 25 commits into from
Closed
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
312bab3
add validator registration endpoint and validator registration service
realbigsean May 13, 2022
49fea56
Add validator registration service and signing domain
realbigsean May 17, 2022
9b8cdda
Remove val registration service and cache from BN, use val registrati…
realbigsean May 19, 2022
c591e7a
update signing
realbigsean May 19, 2022
2ab3f0a
update proposer prep service to cache sent registrations and sign/sen…
realbigsean May 19, 2022
10a03b5
remove bitmask logic, hard code instead
realbigsean May 19, 2022
00c8957
Merge branch 'unstable' of https://github.com/sigp/lighthouse into re…
realbigsean May 20, 2022
bddf03a
add test
realbigsean May 20, 2022
e3a7033
fix signing
realbigsean May 23, 2022
62a6cd8
allow mock el dead code
realbigsean May 23, 2022
291895d
Update beacon_node/http_api/tests/tests.rs
realbigsean May 24, 2022
885819b
Update consensus/types/src/chain_spec.rs
realbigsean May 24, 2022
edc8dce
Update validator_client/src/preparation_service.rs
realbigsean May 24, 2022
b20d7fb
Update validator_client/src/preparation_service.rs
realbigsean May 24, 2022
ca567fd
Update validator_client/src/preparation_service.rs
realbigsean May 24, 2022
b3ee9b8
actually do bitmask operation in test
realbigsean May 24, 2022
bdbe795
don't double publish changed keys on epoch boundary
realbigsean May 24, 2022
3f66349
only start the registration service if we have the private transactio…
realbigsean May 24, 2022
ae061f0
publish registrations regardless of how close we are to the merge
realbigsean May 24, 2022
b759e8d
fix lints, commented out tests for future web3signer addition
realbigsean May 24, 2022
054aa0f
supress lint
realbigsean May 25, 2022
faa87d3
Update beacon_node/http_api/src/lib.rs
realbigsean Jun 29, 2022
365e784
Update beacon_node/http_api/src/lib.rs
realbigsean Jun 29, 2022
a73bafe
PR feedback
realbigsean Jun 29, 2022
76e23fc
Merge branch 'register-validator-api' of https://github.com/realbigse…
realbigsean Jun 29, 2022
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
1 change: 1 addition & 0 deletions Cargo.lock

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

1 change: 1 addition & 0 deletions beacon_node/http_api/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ environment = { path = "../../lighthouse/environment" }
tree_hash = "0.4.1"
sensitive_url = { path = "../../common/sensitive_url" }
logging = { path = "../../common/logging" }
serde_json = "1.0.58"

[[test]]
name = "bn_http_api_tests"
Expand Down
77 changes: 75 additions & 2 deletions beacon_node/http_api/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ use types::{
BlindedPayload, CommitteeCache, ConfigAndPreset, Epoch, EthSpec, ForkName, FullPayload,
ProposerPreparationData, ProposerSlashing, RelativeEpoch, Signature, SignedAggregateAndProof,
SignedBeaconBlock, SignedBeaconBlockMerge, SignedBlindedBeaconBlock,
SignedContributionAndProof, SignedVoluntaryExit, Slot, SyncCommitteeMessage,
SyncContributionData,
SignedContributionAndProof, SignedValidatorRegistrationData, SignedVoluntaryExit, Slot,
SyncCommitteeMessage, SyncContributionData,
};
use version::{
add_consensus_version_header, fork_versioned_response, inconsistent_fork_rejection,
Expand Down Expand Up @@ -2444,6 +2444,78 @@ pub fn serve<T: BeaconChainTypes>(
},
);

// POST validator/register_validator
let post_validator_register_validator = eth1_v1
.and(warp::path("validator"))
.and(warp::path("register_validator"))
.and(warp::path::end())
.and(chain_filter.clone())
.and(warp::addr::remote())
.and(log_filter.clone())
.and(warp::body::json())
.and_then(
|chain: Arc<BeaconChain<T>>,
client_addr: Option<SocketAddr>,
log: Logger,
register_val_data: Vec<SignedValidatorRegistrationData>| {
blocking_json_task(move || {
let execution_layer = chain
.execution_layer
.as_ref()
.ok_or(BeaconChainError::ExecutionLayerMissing)
.map_err(warp_utils::reject::beacon_chain_error)?;
let current_epoch = chain
.epoch()
.map_err(warp_utils::reject::beacon_chain_error)?;
realbigsean marked this conversation as resolved.
Show resolved Hide resolved

debug!(
log,
"Received register validator request";
"count" => register_val_data.len(),
"client" => client_addr
.map(|a| a.to_string())
.unwrap_or_else(|| "unknown".to_string()),
Copy link
Member

Choose a reason for hiding this comment

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

I'm interested in the motivation behind this. This would put validator IP info in our DEBG logs, which I don't think we usually do. I'm not against it if it's going to be useful.

Copy link
Member Author

Choose a reason for hiding this comment

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

This exists in the prepare_beacon_proposer, not sure the reasoning behind it being put there, but I can remove it from both if we'd like?

Copy link
Member

Choose a reason for hiding this comment

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

Oh interesting, I missed this on a review from an external contributor. Let's pull them both out if you don't mind.

);

let preparation_data = register_val_data
.iter()
.filter_map(|register_data| {
chain
.validator_index(&register_data.message.pubkey)
.ok()
.flatten()
.map(|validator_index| ProposerPreparationData {
validator_index: validator_index as u64,
fee_recipient: register_data.message.fee_recipient,
})
})
.collect::<Vec<_>>();

realbigsean marked this conversation as resolved.
Show resolved Hide resolved
// Update the prepare beacon proposer cache based on this request.
execution_layer
.update_proposer_preparation_blocking(current_epoch, &preparation_data)
.map_err(|_e| {
warp_utils::reject::custom_bad_request(
"error processing proposer preparations".to_string(),
)
})?;

// Call prepare beacon proposer blocking with the latest update in order to make
// sure we have a local payload to fall back to in the event of the blined block
// flow failing.
chain.prepare_beacon_proposer_blocking().map_err(|e| {
warp_utils::reject::custom_bad_request(format!(
"error updating proposer preparations: {:?}",
e
))
})?;

//TODO(sean): In the MEV-boost PR, add a call here to send the update request to the builder

Ok(())
})
},
);
// POST validator/sync_committee_subscriptions
let post_validator_sync_committee_subscriptions = eth1_v1
.and(warp::path("validator"))
Expand Down Expand Up @@ -2985,6 +3057,7 @@ pub fn serve<T: BeaconChainTypes>(
.or(post_validator_beacon_committee_subscriptions.boxed())
.or(post_validator_sync_committee_subscriptions.boxed())
.or(post_validator_prepare_beacon_proposer.boxed())
.or(post_validator_register_validator.boxed())
.or(post_lighthouse_liveness.boxed())
.or(post_lighthouse_database_reconstruct.boxed())
.or(post_lighthouse_database_historical_blocks.boxed()),
Expand Down
76 changes: 76 additions & 0 deletions beacon_node/http_api/tests/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use eth2::{
types::*,
BeaconNodeHttpClient, Error, StatusCode, Timeouts,
};
use execution_layer::test_utils::MockExecutionLayer;
use futures::stream::{Stream, StreamExt};
use futures::FutureExt;
use lighthouse_network::{Enr, EnrExt, PeerId};
Expand All @@ -24,6 +25,7 @@ use task_executor::test_utils::TestRuntime;
use tokio::sync::{mpsc, oneshot};
use tokio::time::Duration;
use tree_hash::TreeHash;
use types::application_domain::ApplicationDomain;
use types::{
AggregateSignature, BeaconState, BitList, Domain, EthSpec, Hash256, Keypair, MainnetEthSpec,
RelativeEpoch, SelectionProof, SignedRoot, Slot,
Expand Down Expand Up @@ -64,6 +66,9 @@ struct ApiTester {
network_rx: mpsc::UnboundedReceiver<NetworkMessage<E>>,
local_enr: Enr,
external_peer_id: PeerId,
// This is never directly accessed, but adding it creates a payload cache, which we use in tests here.
#[allow(dead_code)]
mock_el: Option<MockExecutionLayer<E>>,
_runtime: TestRuntime,
}

Expand All @@ -80,6 +85,7 @@ impl ApiTester {
.spec(spec.clone())
.deterministic_keypairs(VALIDATOR_COUNT)
.fresh_ephemeral_store()
.mock_execution_layer()
.build();

harness.advance_slot();
Expand Down Expand Up @@ -214,6 +220,7 @@ impl ApiTester {
network_rx,
local_enr,
external_peer_id,
mock_el: harness.mock_execution_layer,
_runtime: harness.runtime,
}
}
Expand Down Expand Up @@ -293,6 +300,7 @@ impl ApiTester {
network_rx,
local_enr,
external_peer_id,
mock_el: None,
_runtime: harness.runtime,
}
}
Expand Down Expand Up @@ -2226,6 +2234,66 @@ impl ApiTester {
self
}

pub async fn test_post_validator_register_validator(self) -> Self {
let mut registrations = vec![];
let mut fee_recipients = vec![];

let fork = self.chain.head().unwrap().beacon_state.fork();

for (val_index, keypair) in self.validator_keypairs.iter().enumerate() {
let pubkey = keypair.pk.compress();
let fee_recipient = Address::from_low_u64_be(val_index as u64);

let data = ValidatorRegistrationData {
fee_recipient,
gas_limit: 0,
timestamp: 0,
pubkey,
};
let domain = self.chain.spec.get_domain(
Epoch::new(0),
Domain::ApplicationMask(ApplicationDomain::Builder),
&fork,
Hash256::zero(),
);
let message = data.signing_root(domain);
let signature = keypair.sk.sign(message);

fee_recipients.push(fee_recipient);
registrations.push(SignedValidatorRegistrationData {
message: data,
signature,
});
}

self.client
.post_validator_register_validator(&registrations)
.await
.unwrap();

for (val_index, (_, fee_recipient)) in self
.chain
.head()
.unwrap()
.beacon_state
.validators()
.into_iter()
.zip(fee_recipients.into_iter())
.enumerate()
{
let actual = self
.chain
.execution_layer
.as_ref()
.unwrap()
.get_suggested_fee_recipient(val_index as u64)
.await;
assert_eq!(actual, fee_recipient);
}

self
}

#[cfg(target_os = "linux")]
pub async fn test_get_lighthouse_health(self) -> Self {
self.client.get_lighthouse_health().await.unwrap();
Expand Down Expand Up @@ -2973,6 +3041,14 @@ async fn get_validator_beacon_committee_subscriptions() {
.await;
}

#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn post_validator_register_validator() {
ApiTester::new()
.await
.test_post_validator_register_validator()
.await;
}

#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn lighthouse_endpoints() {
ApiTester::new()
Expand Down
1 change: 1 addition & 0 deletions book/src/api-vc-endpoints.md
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ Typical Responses | 200
"DOMAIN_VOLUNTARY_EXIT": "0x04000000",
"DOMAIN_SELECTION_PROOF": "0x05000000",
"DOMAIN_AGGREGATE_AND_PROOF": "0x06000000",
"DOMAIN_APPLICATION_MASK": "0x00000001",
"MAX_VALIDATORS_PER_COMMITTEE": "2048",
"SLOTS_PER_EPOCH": "32",
"EPOCHS_PER_ETH1_VOTING_PERIOD": "32",
Expand Down
17 changes: 17 additions & 0 deletions common/eth2/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -929,6 +929,23 @@ impl BeaconNodeHttpClient {
Ok(())
}

/// `POST validator/register_validator`
pub async fn post_validator_register_validator(
&self,
registration_data: &[SignedValidatorRegistrationData],
) -> Result<(), Error> {
let mut path = self.eth_path(V1)?;

path.path_segments_mut()
.map_err(|()| Error::InvalidUrl(self.server.clone()))?
.push("validator")
.push("register_validator");

self.post(path, &registration_data).await?;

Ok(())
}

/// `GET config/fork_schedule`
pub async fn get_config_fork_schedule(&self) -> Result<GenericResponse<Vec<Fork>>, Error> {
let mut path = self.eth_path(V1)?;
Expand Down
14 changes: 14 additions & 0 deletions consensus/types/src/application_domain.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
#[derive(Debug, PartialEq, Clone, Copy)]
pub enum ApplicationDomain {
Builder,
}

impl ApplicationDomain {
pub fn get_domain_constant(&self) -> u32 {
match self {
// This value is an application index of 0 with the bitmask applied (so it's equivalent to the bit mask).
// Little endian hex: 0x00000001, Binary: 1000000000000000000000000
ApplicationDomain::Builder => 16777216,
Copy link
Member

Choose a reason for hiding this comment

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

It might be nice to have 16777216 as a constant so we don't repeat it in other places of the code. Might even be able to make this function a const fn?

Copy link
Member Author

Choose a reason for hiding this comment

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

Went with a constant here for now

}
}
}
50 changes: 50 additions & 0 deletions consensus/types/src/chain_spec.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::application_domain::ApplicationDomain;
use crate::*;
use eth2_serde_utils::quoted_u64::MaybeQuoted;
use int_to_bytes::int_to_bytes4;
Expand All @@ -20,6 +21,7 @@ pub enum Domain {
SyncCommittee,
ContributionAndProof,
SyncCommitteeSelectionProof,
ApplicationMask(ApplicationDomain),
}

/// Lighthouse's internal configuration struct.
Expand Down Expand Up @@ -159,6 +161,11 @@ pub struct ChainSpec {
pub attestation_subnet_count: u64,
pub random_subnets_per_validator: u64,
pub epochs_per_random_subnet_subscription: u64,

/*
* Application params
*/
pub(crate) domain_application_mask: u32,
}

impl ChainSpec {
Expand Down Expand Up @@ -326,6 +333,7 @@ impl ChainSpec {
Domain::SyncCommittee => self.domain_sync_committee,
Domain::ContributionAndProof => self.domain_contribution_and_proof,
Domain::SyncCommitteeSelectionProof => self.domain_sync_committee_selection_proof,
Domain::ApplicationMask(application_domain) => application_domain.get_domain_constant(),
}
}

Expand Down Expand Up @@ -353,6 +361,17 @@ impl ChainSpec {
self.compute_domain(Domain::Deposit, self.genesis_fork_version, Hash256::zero())
}

// This should be updated to include the current fork and the genesis validators root, but discussion is ongoing:
//
// https://github.com/ethereum/builder-specs/issues/14
pub fn get_builder_domain(&self) -> Hash256 {
self.compute_domain(
Domain::ApplicationMask(ApplicationDomain::Builder),
self.genesis_fork_version,
Hash256::zero(),
)
}

/// Return the 32-byte fork data root for the `current_version` and `genesis_validators_root`.
///
/// This is used primarily in signature domains to avoid collisions across forks/chains.
Expand Down Expand Up @@ -565,6 +584,11 @@ impl ChainSpec {
maximum_gossip_clock_disparity_millis: 500,
target_aggregators_per_committee: 16,
epochs_per_random_subnet_subscription: 256,

/*
* Application specific
*/
domain_application_mask: 16777216, // Little endian hex: 0x00000001, Binary: 1000000000000000000000000
realbigsean marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand Down Expand Up @@ -763,6 +787,11 @@ impl ChainSpec {
maximum_gossip_clock_disparity_millis: 500,
target_aggregators_per_committee: 16,
epochs_per_random_subnet_subscription: 256,

/*
* Application specific
*/
domain_application_mask: 16777216, // Little endian hex: 0x00000001, Binary: 1000000000000000000000000
realbigsean marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
Expand Down Expand Up @@ -1119,6 +1148,27 @@ mod tests {
&spec,
);
test_domain(Domain::SyncCommittee, spec.domain_sync_committee, &spec);

// The builder domain index is zero
let builder_domain_pre_mask = [0; 4];
test_domain(
Domain::ApplicationMask(ApplicationDomain::Builder),
apply_bit_mask(builder_domain_pre_mask, &spec),
&spec,
);
}

fn apply_bit_mask(domain_bytes: [u8; 4], spec: &ChainSpec) -> u32 {
let mut domain = [0; 4];
let mask_bytes = int_to_bytes4(spec.domain_application_mask);

// Apply application bit mask
for (i, (domain_byte, mask_byte)) in domain_bytes.iter().zip(mask_bytes.iter()).enumerate()
{
domain[i] = domain_byte | mask_byte;
}

u32::from_le_bytes(domain)
}

// Test that `fork_name_at_epoch` and `fork_epoch` are consistent.
Expand Down
Loading