Skip to content

Commit 1100fc4

Browse files
s0me0ne-unkn0wngithub-actions[bot]
s0me0ne-unkn0wn
and
github-actions[bot]
authored
Fix executor param fetching session index (#9774)
This aims to fix #4292 We have three cases of candidate validation where a proper set of executor environment parameters should be used: 1) Backing, and we're currently doing the right thing, requesting the executor params at the relay parent at which the candidate was produced: https://github.com/paritytech/polkadot-sdk/blob/e7f36ab82934a7142f3ebd7f8b5566f12f85339b/polkadot/node/core/backing/src/lib.rs#L1140-L1146 2) Approval voting, where a wrong session was used, and this PR fixes that; 3) Disputes, where the session index, again, is hopefully derived from the relay parent at which the candidate was produced: https://github.com/paritytech/polkadot-sdk/blob/63958c454643ddafdde8be17af5334aa95954550/polkadot/node/subsystem-types/src/messages.rs#L295-L296 So, hopefully, this PR fixes the only wrong case and harmonizes the executor param fetching over all the existing use cases. --------- Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>
1 parent c223162 commit 1100fc4

File tree

4 files changed

+149
-31
lines changed

4 files changed

+149
-31
lines changed

‎polkadot/node/core/approval-voting/src/import.rs‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ use super::approval_db::v3;
6161
use crate::{
6262
backend::{Backend, OverlayedBackend},
6363
criteria::{AssignmentCriteria, OurAssignment},
64-
get_extended_session_info, get_session_info,
64+
get_extended_session_info_by_index, get_session_info_by_index,
6565
persisted_entries::CandidateEntry,
6666
};
6767

@@ -220,7 +220,8 @@ async fn imported_block_info<Sender: SubsystemSender<RuntimeApiMessage>>(
220220
};
221221

222222
let extended_session_info =
223-
get_extended_session_info(env.runtime_info, sender, block_hash, session_index).await;
223+
get_extended_session_info_by_index(env.runtime_info, sender, block_hash, session_index)
224+
.await;
224225
let enable_v2_assignments = extended_session_info.map_or(false, |extended_session_info| {
225226
*extended_session_info
226227
.node_features
@@ -229,9 +230,10 @@ async fn imported_block_info<Sender: SubsystemSender<RuntimeApiMessage>>(
229230
.unwrap_or(&false)
230231
});
231232

232-
let session_info = get_session_info(env.runtime_info, sender, block_hash, session_index)
233-
.await
234-
.ok_or(ImportedBlockInfoError::SessionInfoUnavailable)?;
233+
let session_info =
234+
get_session_info_by_index(env.runtime_info, sender, block_hash, session_index)
235+
.await
236+
.ok_or(ImportedBlockInfoError::SessionInfoUnavailable)?;
235237

236238
gum::debug!(target: LOG_TARGET, ?enable_v2_assignments, "V2 assignments");
237239
let (assignments, slot, relay_vrf_story) = {
@@ -456,7 +458,9 @@ pub(crate) async fn handle_new_head<
456458
} = imported_block_info;
457459

458460
let session_info =
459-
match get_session_info(session_info_provider, sender, head, session_index).await {
461+
match get_session_info_by_index(session_info_provider, sender, head, session_index)
462+
.await
463+
{
460464
Some(session_info) => session_info,
461465
None => return Ok(Vec::new()),
462466
};

‎polkadot/node/core/approval-voting/src/lib.rs‎

Lines changed: 55 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -851,6 +851,27 @@ async fn get_extended_session_info<'a, Sender>(
851851
runtime_info: &'a mut RuntimeInfo,
852852
sender: &mut Sender,
853853
relay_parent: Hash,
854+
) -> Option<&'a ExtendedSessionInfo>
855+
where
856+
Sender: SubsystemSender<RuntimeApiMessage>,
857+
{
858+
match runtime_info.get_session_info(sender, relay_parent).await {
859+
Ok(extended_info) => Some(&extended_info),
860+
Err(_) => {
861+
gum::debug!(
862+
target: LOG_TARGET,
863+
?relay_parent,
864+
"Can't obtain SessionInfo or ExecutorParams"
865+
);
866+
None
867+
},
868+
}
869+
}
870+
871+
async fn get_extended_session_info_by_index<'a, Sender>(
872+
runtime_info: &'a mut RuntimeInfo,
873+
sender: &mut Sender,
874+
relay_parent: Hash,
854875
session_index: SessionIndex,
855876
) -> Option<&'a ExtendedSessionInfo>
856877
where
@@ -873,7 +894,7 @@ where
873894
}
874895
}
875896

876-
async fn get_session_info<'a, Sender>(
897+
async fn get_session_info_by_index<'a, Sender>(
877898
runtime_info: &'a mut RuntimeInfo,
878899
sender: &mut Sender,
879900
relay_parent: Hash,
@@ -882,7 +903,7 @@ async fn get_session_info<'a, Sender>(
882903
where
883904
Sender: SubsystemSender<RuntimeApiMessage>,
884905
{
885-
get_extended_session_info(runtime_info, sender, relay_parent, session_index)
906+
get_extended_session_info_by_index(runtime_info, sender, relay_parent, session_index)
886907
.await
887908
.map(|extended_info| &extended_info.session_info)
888909
}
@@ -976,7 +997,7 @@ impl State {
976997
where
977998
Sender: SubsystemSender<RuntimeApiMessage>,
978999
{
979-
let session_info = match get_session_info(
1000+
let session_info = match get_session_info_by_index(
9801001
session_info_provider,
9811002
sender,
9821003
block_entry.parent_hash(),
@@ -1901,8 +1922,10 @@ async fn distribution_messages_for_activation<Sender: SubsystemSender<RuntimeApi
19011922
match get_extended_session_info(
19021923
session_info_provider,
19031924
sender,
1904-
block_entry.block_hash(),
1905-
block_entry.session(),
1925+
candidate_entry
1926+
.candidate_receipt()
1927+
.descriptor()
1928+
.relay_parent(),
19061929
)
19071930
.await
19081931
{
@@ -2686,7 +2709,7 @@ where
26862709
)),
26872710
};
26882711

2689-
let session_info = match get_session_info(
2712+
let session_info = match get_session_info_by_index(
26902713
session_info_provider,
26912714
sender,
26922715
block_entry.parent_hash(),
@@ -3282,24 +3305,21 @@ async fn process_wakeup<Sender: SubsystemSender<RuntimeApiMessage>>(
32823305
_ => return Ok(Vec::new()),
32833306
};
32843307

3285-
let ExtendedSessionInfo { ref session_info, ref executor_params, .. } =
3286-
match get_extended_session_info(
3287-
session_info_provider,
3288-
sender,
3289-
block_entry.block_hash(),
3290-
block_entry.session(),
3291-
)
3292-
.await
3293-
{
3294-
Some(i) => i,
3295-
None => return Ok(Vec::new()),
3296-
};
3308+
let (no_show_slots, needed_approvals) = match get_session_info_by_index(
3309+
session_info_provider,
3310+
sender,
3311+
block_entry.block_hash(),
3312+
block_entry.session(),
3313+
)
3314+
.await
3315+
{
3316+
Some(i) => (i.no_show_slots, i.needed_approvals),
3317+
None => return Ok(Vec::new()),
3318+
};
32973319

32983320
let block_tick = slot_number_to_tick(state.slot_duration_millis, block_entry.slot());
3299-
let no_show_duration = slot_number_to_tick(
3300-
state.slot_duration_millis,
3301-
Slot::from(u64::from(session_info.no_show_slots)),
3302-
);
3321+
let no_show_duration =
3322+
slot_number_to_tick(state.slot_duration_millis, Slot::from(u64::from(no_show_slots)));
33033323
let tranche_now = state.clock.tranche_now(state.slot_duration_millis, block_entry.slot());
33043324

33053325
gum::trace!(
@@ -3322,7 +3342,7 @@ async fn process_wakeup<Sender: SubsystemSender<RuntimeApiMessage>>(
33223342
tranche_now,
33233343
block_tick,
33243344
no_show_duration,
3325-
session_info.needed_approvals as _,
3345+
needed_approvals as _,
33263346
);
33273347

33283348
let should_trigger = should_trigger_assignment(
@@ -3357,6 +3377,16 @@ async fn process_wakeup<Sender: SubsystemSender<RuntimeApiMessage>>(
33573377
};
33583378

33593379
if let Some((cert, val_index, tranche)) = maybe_cert {
3380+
let ExtendedSessionInfo { ref executor_params, .. } = match get_extended_session_info(
3381+
session_info_provider,
3382+
sender,
3383+
candidate_entry.candidate_receipt().descriptor().relay_parent(),
3384+
)
3385+
.await
3386+
{
3387+
Some(i) => i,
3388+
None => return Ok(actions),
3389+
};
33603390
let indirect_cert =
33613391
IndirectAssignmentCertV2 { block_hash: relay_block, validator: val_index, cert };
33623392

@@ -3742,7 +3772,7 @@ async fn issue_approval<
37423772
},
37433773
};
37443774

3745-
let session_info = match get_session_info(
3775+
let session_info = match get_session_info_by_index(
37463776
session_info_provider,
37473777
sender,
37483778
block_entry.parent_hash(),
@@ -3868,7 +3898,7 @@ async fn maybe_create_signature<
38683898
None => return Ok(sign_no_later_then),
38693899
};
38703900

3871-
let session_info = match get_session_info(
3901+
let session_info = match get_session_info_by_index(
38723902
session_info_provider,
38733903
sender,
38743904
block_entry.parent_hash(),

‎polkadot/node/core/approval-voting/src/tests.rs‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1105,6 +1105,23 @@ async fn import_block(
11051105
);
11061106
}
11071107
}
1108+
1109+
match overseer.peek().timeout(Duration::from_millis(50)).await.flatten() {
1110+
Some(AllMessages::RuntimeApi(RuntimeApiMessage::Request(
1111+
_,
1112+
RuntimeApiRequest::SessionIndexForChild(_),
1113+
))) => {
1114+
let AllMessages::RuntimeApi(RuntimeApiMessage::Request(
1115+
_,
1116+
RuntimeApiRequest::SessionIndexForChild(s_tx),
1117+
)) = overseer.recv().await
1118+
else {
1119+
panic!("Unexpected message");
1120+
};
1121+
s_tx.send(Ok(1u32.into())).unwrap();
1122+
},
1123+
_ => (),
1124+
}
11081125
}
11091126

11101127
#[test]
@@ -2946,6 +2963,13 @@ fn subsystem_validate_approvals_cache() {
29462963
assert!(clock.inner.lock().current_wakeup_is(slot_to_tick(slot)));
29472964
clock.inner.lock().wakeup_all(slot_to_tick(slot));
29482965

2966+
assert_matches!(
2967+
overseer_recv(&mut virtual_overseer).await,
2968+
AllMessages::RuntimeApi(RuntimeApiMessage::Request(_, RuntimeApiRequest::SessionIndexForChild(rx), )) => {
2969+
rx.send(Ok(1u32.into())).unwrap();
2970+
}
2971+
);
2972+
29492973
futures_timer::Delay::new(Duration::from_millis(200)).await;
29502974

29512975
clock.inner.lock().wakeup_all(slot_to_tick(slot + 2));
@@ -3370,6 +3394,24 @@ where
33703394
clock.inner.lock().set_tick(tick);
33713395
futures_timer::Delay::new(Duration::from_millis(100)).await;
33723396

3397+
match virtual_overseer.peek().timeout(Duration::from_millis(50)).await.flatten() {
3398+
Some(AllMessages::RuntimeApi(RuntimeApiMessage::Request(
3399+
_,
3400+
RuntimeApiRequest::SessionIndexForChild(_),
3401+
))) => {
3402+
let AllMessages::RuntimeApi(RuntimeApiMessage::Request(
3403+
_,
3404+
RuntimeApiRequest::SessionIndexForChild(rx),
3405+
)) = virtual_overseer.recv().await
3406+
else {
3407+
panic!("Unexpected message");
3408+
};
3409+
rx.send(Ok(1u32.into())).unwrap();
3410+
futures_timer::Delay::new(Duration::from_millis(100)).await;
3411+
},
3412+
_ => (),
3413+
}
3414+
33733415
// Assert that Alice's assignment is triggered at the correct tick.
33743416
let candidate_entry = store.load_candidate_entry(&candidate_hash).unwrap().unwrap();
33753417
let our_assignment =
@@ -4048,6 +4090,13 @@ fn test_approval_is_sent_on_max_approval_coalesce_count() {
40484090
assert!(clock.inner.lock().current_wakeup_is(slot_to_tick(slot)));
40494091
clock.inner.lock().wakeup_all(slot_to_tick(slot));
40504092

4093+
assert_matches!(
4094+
overseer_recv(&mut virtual_overseer).await,
4095+
AllMessages::RuntimeApi(RuntimeApiMessage::Request(_, RuntimeApiRequest::SessionIndexForChild(rx), )) => {
4096+
rx.send(Ok(1u32.into())).unwrap();
4097+
}
4098+
);
4099+
40514100
futures_timer::Delay::new(Duration::from_millis(200)).await;
40524101

40534102
clock.inner.lock().wakeup_all(slot_to_tick(slot + 2));
@@ -4349,6 +4398,13 @@ fn test_approval_is_sent_on_max_approval_coalesce_wait() {
43494398
assert!(clock.inner.lock().current_wakeup_is(slot_to_tick(slot)));
43504399
clock.inner.lock().wakeup_all(slot_to_tick(slot));
43514400

4401+
assert_matches!(
4402+
overseer_recv(&mut virtual_overseer).await,
4403+
AllMessages::RuntimeApi(RuntimeApiMessage::Request(_, RuntimeApiRequest::SessionIndexForChild(rx), )) => {
4404+
rx.send(Ok(1u32.into())).unwrap();
4405+
}
4406+
);
4407+
43524408
futures_timer::Delay::new(Duration::from_millis(200)).await;
43534409

43544410
clock.inner.lock().wakeup_all(slot_to_tick(slot + 2));
@@ -4463,6 +4519,13 @@ async fn setup_overseer_with_two_blocks_each_with_one_assignment_triggered(
44634519
assert!(clock.inner.lock().current_wakeup_is(slot_to_tick(slot)));
44644520
clock.inner.lock().wakeup_all(slot_to_tick(slot));
44654521

4522+
assert_matches!(
4523+
overseer_recv(virtual_overseer).await,
4524+
AllMessages::RuntimeApi(RuntimeApiMessage::Request(_, RuntimeApiRequest::SessionIndexForChild(rx), )) => {
4525+
rx.send(Ok(1u32.into())).unwrap();
4526+
}
4527+
);
4528+
44664529
futures_timer::Delay::new(Duration::from_millis(200)).await;
44674530

44684531
clock.inner.lock().wakeup_all(slot_to_tick(slot + 2));
@@ -4566,6 +4629,13 @@ async fn setup_overseer_with_blocks_with_two_assignments_triggered(
45664629
assert!(clock.inner.lock().current_wakeup_is(slot_to_tick(slot)));
45674630
clock.inner.lock().wakeup_all(slot_to_tick(slot));
45684631

4632+
assert_matches!(
4633+
overseer_recv(virtual_overseer).await,
4634+
AllMessages::RuntimeApi(RuntimeApiMessage::Request(_, RuntimeApiRequest::SessionIndexForChild(rx), )) => {
4635+
rx.send(Ok(1u32.into())).unwrap();
4636+
}
4637+
);
4638+
45694639
futures_timer::Delay::new(Duration::from_millis(200)).await;
45704640

45714641
clock.inner.lock().wakeup_all(slot_to_tick(slot + 2));
@@ -5551,6 +5621,12 @@ fn subsystem_launches_missed_assignments_on_restart() {
55515621
}
55525622
);
55535623

5624+
assert_matches!(
5625+
overseer_recv(&mut virtual_overseer).await,
5626+
AllMessages::RuntimeApi(RuntimeApiMessage::Request(_, RuntimeApiRequest::SessionIndexForChild(rx), )) => {
5627+
rx.send(Ok(1u32.into())).unwrap();
5628+
}
5629+
);
55545630
assert_matches!(
55555631
overseer_recv(&mut virtual_overseer).await,
55565632
AllMessages::ApprovalDistribution(ApprovalDistributionMessage::DistributeAssignment(

‎prdoc/pr_9774.prdoc‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
title: Fix executor param fetching session index
2+
doc:
3+
- audience: Node Dev
4+
description: |-
5+
Fix executor environment parameters fetching inconsistency
6+
crates:
7+
- name: polkadot-node-core-approval-voting
8+
bump: patch

0 commit comments

Comments
 (0)