Skip to content

Commit f4b1973

Browse files
michaelsproulpawanjay176
authored andcommitted
Optimise no-op PATCH ops in VC HTTP API (sigp#5064)
* Optimise no-op changes in VC API * Handle another no-op case * Merge remote-tracking branch 'origin/unstable' into opt-vc-patch-api
1 parent 9ea4e63 commit f4b1973

File tree

1 file changed

+61
-19
lines changed
  • validator_client/src/http_api

1 file changed

+61
-19
lines changed

validator_client/src/http_api/mod.rs

Lines changed: 61 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -685,7 +685,16 @@ pub fn serve<T: 'static + SlotClock + Clone, E: EthSpec>(
685685

686686
let maybe_graffiti = body.graffiti.clone().map(Into::into);
687687
let initialized_validators_rw_lock = validator_store.initialized_validators();
688-
let mut initialized_validators = initialized_validators_rw_lock.write();
688+
let initialized_validators = initialized_validators_rw_lock.upgradable_read();
689+
690+
// Do not make any changes if all fields are identical or unchanged.
691+
fn equal_or_none<T: PartialEq>(
692+
current_value: Option<T>,
693+
new_value: Option<T>,
694+
) -> bool {
695+
new_value.is_none() || current_value == new_value
696+
}
697+
689698
match (
690699
initialized_validators.is_enabled(&validator_pubkey),
691700
initialized_validators.validator(&validator_pubkey.compress()),
@@ -694,32 +703,65 @@ pub fn serve<T: 'static + SlotClock + Clone, E: EthSpec>(
694703
"no validator for {:?}",
695704
validator_pubkey
696705
))),
706+
// If all specified parameters match their existing settings, then this
707+
// change is a no-op.
697708
(Some(is_enabled), Some(initialized_validator))
698-
if Some(is_enabled) == body.enabled
699-
&& initialized_validator.get_gas_limit() == body.gas_limit
700-
&& initialized_validator.get_builder_boost_factor()
701-
== body.builder_boost_factor
702-
&& initialized_validator.get_builder_proposals()
703-
== body.builder_proposals
704-
&& initialized_validator.get_prefer_builder_proposals()
705-
== body.prefer_builder_proposals
706-
&& initialized_validator.get_graffiti() == maybe_graffiti =>
709+
if equal_or_none(Some(is_enabled), body.enabled)
710+
&& equal_or_none(
711+
initialized_validator.get_gas_limit(),
712+
body.gas_limit,
713+
)
714+
&& equal_or_none(
715+
initialized_validator.get_builder_boost_factor(),
716+
body.builder_boost_factor,
717+
)
718+
&& equal_or_none(
719+
initialized_validator.get_builder_proposals(),
720+
body.builder_proposals,
721+
)
722+
&& equal_or_none(
723+
initialized_validator.get_prefer_builder_proposals(),
724+
body.prefer_builder_proposals,
725+
)
726+
&& equal_or_none(
727+
initialized_validator.get_graffiti(),
728+
maybe_graffiti,
729+
) =>
730+
{
731+
Ok(())
732+
}
733+
// Disabling an already disabled validator *with no other changes* is a
734+
// no-op.
735+
(Some(false), None)
736+
if body.enabled.map_or(true, |enabled| !enabled)
737+
&& body.gas_limit.is_none()
738+
&& body.builder_boost_factor.is_none()
739+
&& body.builder_proposals.is_none()
740+
&& body.prefer_builder_proposals.is_none()
741+
&& maybe_graffiti.is_none() =>
707742
{
708743
Ok(())
709744
}
710745
(Some(_), _) => {
746+
// Upgrade read lock only in the case where a write is actually
747+
// required.
748+
let mut initialized_validators_write =
749+
parking_lot::RwLockUpgradableReadGuard::upgrade(
750+
initialized_validators,
751+
);
711752
if let Some(handle) = task_executor.handle() {
712753
handle
713754
.block_on(
714-
initialized_validators.set_validator_definition_fields(
715-
&validator_pubkey,
716-
body.enabled,
717-
body.gas_limit,
718-
body.builder_proposals,
719-
body.builder_boost_factor,
720-
body.prefer_builder_proposals,
721-
body.graffiti,
722-
),
755+
initialized_validators_write
756+
.set_validator_definition_fields(
757+
&validator_pubkey,
758+
body.enabled,
759+
body.gas_limit,
760+
body.builder_proposals,
761+
body.builder_boost_factor,
762+
body.prefer_builder_proposals,
763+
body.graffiti,
764+
),
723765
)
724766
.map_err(|e| {
725767
warp_utils::reject::custom_server_error(format!(

0 commit comments

Comments
 (0)