Skip to content

Commit

Permalink
Merge remote-tracking branch 'origin/master' into bkchr-expose-max-tr…
Browse files Browse the repository at this point in the history
…ansact-weight
  • Loading branch information
bkchr committed Apr 18, 2024
2 parents 14cb411 + 88a2f36 commit f72e3c0
Show file tree
Hide file tree
Showing 14 changed files with 475 additions and 190 deletions.
3 changes: 3 additions & 0 deletions .github/workflows/test-github-actions.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ concurrency:
group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }}
cancel-in-progress: true

env:
CARGO_NET_GIT_FETCH_WITH_CLI: true

jobs:
test-linux-stable-int:
runs-on: arc-runners-polkadot-sdk
Expand Down
51 changes: 51 additions & 0 deletions polkadot/node/core/approval-voting/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -978,6 +978,7 @@ where
woken_block,
woken_candidate,
&subsystem.metrics,
&wakeups,
).await?
}
next_msg = ctx.recv().fuse() => {
Expand Down Expand Up @@ -1152,6 +1153,7 @@ async fn handle_actions<Context>(
candidate_hash,
delayed_approvals_timers,
approval_request,
&wakeups,
)
.await?
.into_iter()
Expand Down Expand Up @@ -1663,6 +1665,7 @@ async fn handle_from_overseer<Context>(
|r| {
let _ = res.send(r);
},
&wakeups,
)
.await?
.0,
Expand Down Expand Up @@ -2477,6 +2480,7 @@ async fn check_and_import_approval<T, Sender>(
metrics: &Metrics,
approval: IndirectSignedApprovalVoteV2,
with_response: impl FnOnce(ApprovalCheckResult) -> T,
wakeups: &Wakeups,
) -> SubsystemResult<(Vec<Action>, T)>
where
Sender: SubsystemSender<RuntimeApiMessage>,
Expand Down Expand Up @@ -2655,6 +2659,7 @@ where
approved_candidate_hash,
candidate_entry,
ApprovalStateTransition::RemoteApproval(approval.validator),
wakeups,
)
.await;
actions.extend(new_actions);
Expand Down Expand Up @@ -2689,6 +2694,10 @@ impl ApprovalStateTransition {
ApprovalStateTransition::WakeupProcessed => false,
}
}

fn is_remote_approval(&self) -> bool {
matches!(*self, ApprovalStateTransition::RemoteApproval(_))
}
}

// Advance the approval state, either by importing an approval vote which is already checked to be
Expand All @@ -2705,6 +2714,7 @@ async fn advance_approval_state<Sender>(
candidate_hash: CandidateHash,
mut candidate_entry: CandidateEntry,
transition: ApprovalStateTransition,
wakeups: &Wakeups,
) -> Vec<Action>
where
Sender: SubsystemSender<RuntimeApiMessage>,
Expand Down Expand Up @@ -2835,6 +2845,43 @@ where
status.required_tranches,
));

if is_approved && transition.is_remote_approval() {
// Make sure we wake other blocks in case they have
// a no-show that might be covered by this approval.
for (fork_block_hash, fork_approval_entry) in candidate_entry
.block_assignments
.iter()
.filter(|(hash, _)| **hash != block_hash)
{
let assigned_on_fork_block = validator_index
.as_ref()
.map(|validator_index| fork_approval_entry.is_assigned(*validator_index))
.unwrap_or_default();
if wakeups.wakeup_for(*fork_block_hash, candidate_hash).is_none() &&
!fork_approval_entry.is_approved() &&
assigned_on_fork_block
{
let fork_block_entry = db.load_block_entry(fork_block_hash);
if let Ok(Some(fork_block_entry)) = fork_block_entry {
actions.push(Action::ScheduleWakeup {
block_hash: *fork_block_hash,
block_number: fork_block_entry.block_number(),
candidate_hash,
// Schedule the wakeup next tick, since the assignment must be a
// no-show, because there is no-wakeup scheduled.
tick: tick_now + 1,
})
} else {
gum::debug!(
target: LOG_TARGET,
?fork_block_entry,
?fork_block_hash,
"Failed to load block entry"
)
}
}
}
}
// We have no need to write the candidate entry if all of the following
// is true:
//
Expand Down Expand Up @@ -2896,6 +2943,7 @@ async fn process_wakeup<Context>(
relay_block: Hash,
candidate_hash: CandidateHash,
metrics: &Metrics,
wakeups: &Wakeups,
) -> SubsystemResult<Vec<Action>> {
let mut span = state
.spans
Expand Down Expand Up @@ -3064,6 +3112,7 @@ async fn process_wakeup<Context>(
candidate_hash,
candidate_entry,
ApprovalStateTransition::WakeupProcessed,
wakeups,
)
.await,
);
Expand Down Expand Up @@ -3294,6 +3343,7 @@ async fn issue_approval<Context>(
candidate_hash: CandidateHash,
delayed_approvals_timers: &mut DelayedApprovalTimer,
ApprovalVoteRequest { validator_index, block_hash }: ApprovalVoteRequest,
wakeups: &Wakeups,
) -> SubsystemResult<Vec<Action>> {
let mut issue_approval_span = state
.spans
Expand Down Expand Up @@ -3415,6 +3465,7 @@ async fn issue_approval<Context>(
candidate_hash,
candidate_entry,
ApprovalStateTransition::LocalApproval(validator_index as _),
wakeups,
)
.await;

Expand Down
182 changes: 181 additions & 1 deletion polkadot/node/core/approval-voting/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -834,7 +834,6 @@ impl ChainBuilder {
cur_hash = cur_header.parent_hash;
}
ancestry.reverse();

import_block(
overseer,
ancestry.as_ref(),
Expand Down Expand Up @@ -1922,6 +1921,187 @@ fn subsystem_assignment_import_updates_candidate_entry_and_schedules_wakeup() {
});
}

#[test]
fn subsystem_always_has_a_wakeup_when_pending() {
// Approvals sent after all assignments are no-show, the approval
// should be counted on the fork relay chain on the next tick.
test_approvals_on_fork_are_always_considered_after_no_show(
30,
vec![(29, false), (30, false), (31, true)],
);
// Approvals sent before fork no-shows, the approval
// should be counted on the fork relay chain when it no-shows.
test_approvals_on_fork_are_always_considered_after_no_show(
8, // a tick smaller than the no-show tick which is 30.
vec![(7, false), (8, false), (29, false), (30, true), (31, true)],
);
}

fn test_approvals_on_fork_are_always_considered_after_no_show(
tick_to_send_approval: Tick,
expected_approval_status: Vec<(Tick, bool)>,
) {
let config = HarnessConfig::default();
let store = config.backend();

test_harness(config, |test_harness| async move {
let TestHarness {
mut virtual_overseer,
clock,
sync_oracle_handle: _sync_oracle_handle,
..
} = test_harness;

assert_matches!(
overseer_recv(&mut virtual_overseer).await,
AllMessages::ChainApi(ChainApiMessage::FinalizedBlockNumber(rx)) => {
rx.send(Ok(0)).unwrap();
}
);
let candidate_hash = Hash::repeat_byte(0x04);

let candidate_descriptor = make_candidate(ParaId::from(1_u32), &candidate_hash);
let candidate_hash = candidate_descriptor.hash();

let block_hash = Hash::repeat_byte(0x01);
let block_hash_fork = Hash::repeat_byte(0x02);

let candidate_index = 0;
let validator = ValidatorIndex(0);
let validators = vec![
Sr25519Keyring::Alice,
Sr25519Keyring::Bob,
Sr25519Keyring::Charlie,
Sr25519Keyring::Dave,
Sr25519Keyring::Eve,
];
// Add block hash 0x01 and for 0x02
ChainBuilder::new()
.add_block(
block_hash,
ChainBuilder::GENESIS_HASH,
1,
BlockConfig {
slot: Slot::from(1),
candidates: Some(vec![(
candidate_descriptor.clone(),
CoreIndex(0),
GroupIndex(0),
)]),
session_info: Some(SessionInfo {
validator_groups: IndexedVec::<GroupIndex, Vec<ValidatorIndex>>::from(
vec![
vec![ValidatorIndex(0), ValidatorIndex(1)],
vec![ValidatorIndex(2)],
vec![ValidatorIndex(3), ValidatorIndex(4)],
],
),
needed_approvals: 1,
..session_info(&validators)
}),
end_syncing: false,
},
)
.add_block(
block_hash_fork,
ChainBuilder::GENESIS_HASH,
1,
BlockConfig {
slot: Slot::from(1),
candidates: Some(vec![(candidate_descriptor, CoreIndex(0), GroupIndex(0))]),
session_info: Some(SessionInfo {
validator_groups: IndexedVec::<GroupIndex, Vec<ValidatorIndex>>::from(
vec![
vec![ValidatorIndex(0), ValidatorIndex(1)],
vec![ValidatorIndex(2)],
vec![ValidatorIndex(3), ValidatorIndex(4)],
],
),
needed_approvals: 1,
..session_info(&validators)
}),
end_syncing: false,
},
)
.build(&mut virtual_overseer)
.await;

// Send assignments for the same candidate on both forks
let rx = check_and_import_assignment(
&mut virtual_overseer,
block_hash,
candidate_index,
validator,
)
.await;
assert_eq!(rx.await, Ok(AssignmentCheckResult::Accepted));

let rx = check_and_import_assignment(
&mut virtual_overseer,
block_hash_fork,
candidate_index,
validator,
)
.await;

assert_eq!(rx.await, Ok(AssignmentCheckResult::Accepted));
// Wake on APPROVAL_DELAY first
assert!(clock.inner.lock().current_wakeup_is(2));
clock.inner.lock().set_tick(2);
futures_timer::Delay::new(Duration::from_millis(100)).await;

// Wake up on no-show
assert!(clock.inner.lock().current_wakeup_is(30));

for (tick, status) in expected_approval_status
.iter()
.filter(|(tick, _)| *tick < tick_to_send_approval)
{
// Wake up on no-show
clock.inner.lock().set_tick(*tick);
futures_timer::Delay::new(Duration::from_millis(100)).await;
let block_entry = store.load_block_entry(&block_hash).unwrap().unwrap();
let block_entry_fork = store.load_block_entry(&block_hash_fork).unwrap().unwrap();
assert!(!block_entry.is_fully_approved());
assert_eq!(block_entry_fork.is_fully_approved(), *status);
}

clock.inner.lock().set_tick(tick_to_send_approval);
futures_timer::Delay::new(Duration::from_millis(100)).await;

// Send the approval for candidate just in the context of 0x01 block.
let rx = check_and_import_approval(
&mut virtual_overseer,
block_hash,
candidate_index,
validator,
candidate_hash,
1,
false,
None,
)
.await;

assert_eq!(rx.await, Ok(ApprovalCheckResult::Accepted),);

// Check approval status for the fork_block is correctly transitioned.
for (tick, status) in expected_approval_status
.iter()
.filter(|(tick, _)| *tick >= tick_to_send_approval)
{
// Wake up on no-show
clock.inner.lock().set_tick(*tick);
futures_timer::Delay::new(Duration::from_millis(100)).await;
let block_entry = store.load_block_entry(&block_hash).unwrap().unwrap();
let block_entry_fork = store.load_block_entry(&block_hash_fork).unwrap().unwrap();
assert!(block_entry.is_fully_approved());
assert_eq!(block_entry_fork.is_fully_approved(), *status);
}

virtual_overseer
});
}

#[test]
fn subsystem_process_wakeup_schedules_wakeup() {
test_harness(HarnessConfig::default(), |test_harness| async move {
Expand Down
6 changes: 3 additions & 3 deletions polkadot/node/core/chain-selection/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -619,7 +619,7 @@ async fn handle_active_leaf(

// Extract all reversion logs from a header in ascending order.
//
// Ignores logs with number >= the block header number.
// Ignores logs with number > the block header number.
fn extract_reversion_logs(header: &Header) -> Vec<BlockNumber> {
let number = header.number;
let mut logs = header
Expand All @@ -639,14 +639,14 @@ fn extract_reversion_logs(header: &Header) -> Vec<BlockNumber> {

None
},
Ok(Some(ConsensusLog::Revert(b))) if b < number => Some(b),
Ok(Some(ConsensusLog::Revert(b))) if b <= number => Some(b),
Ok(Some(ConsensusLog::Revert(b))) => {
gum::warn!(
target: LOG_TARGET,
revert_target = b,
block_number = number,
block_hash = ?header.hash(),
"Block issued invalid revert digest targeting itself or future"
"Block issued invalid revert digest targeting future"
);

None
Expand Down
Loading

0 comments on commit f72e3c0

Please sign in to comment.