Send import notification always for re-orgs (#7118)

* Send import notification always for re-orgs

This pr changes the behavior of sending import notifications. Before we
only send notifications when importing blocks on the tip of the chain or
on similar conditions. However we did not send a notification when we
for example being in a state where we import multiple blocks to catch
up. If we re-org in this process, systems like the transaction pool
would not be notified about this re-org. This means, that we would also
not resubmit transactions of these retracted blocks. This pr fixes this,
by always sending a notification on a re-org.

See
https://github.com/substrate-developer-hub/substrate-node-template/issues/82
for some context about the bug.

* Update client/service/src/client/client.rs

Co-authored-by: André Silva <123550+andresilva@users.noreply.github.com>

Co-authored-by: André Silva <123550+andresilva@users.noreply.github.com>
This commit is contained in:
Bastian Köcher
2020-09-16 16:43:43 +02:00
committed by GitHub
parent 83bde4c1e6
commit 0a6f7b08d7
2 changed files with 56 additions and 1 deletions
@@ -1802,3 +1802,57 @@ fn cleans_up_closed_notification_sinks_on_block_import() {
assert_eq!(client.finality_notification_sinks().lock().len(), 0);
}
/// Test that ensures that we always send an import notification for re-orgs.
#[test]
fn reorg_triggers_a_notification_even_for_sources_that_should_not_trigger_notifications() {
let mut client = TestClientBuilder::new().build();
let mut notification_stream = futures::executor::block_on_stream(
client.import_notification_stream()
);
let a1 = client.new_block_at(
&BlockId::Number(0),
Default::default(),
false,
).unwrap().build().unwrap().block;
client.import(BlockOrigin::NetworkInitialSync, a1.clone()).unwrap();
let a2 = client.new_block_at(
&BlockId::Hash(a1.hash()),
Default::default(),
false,
).unwrap().build().unwrap().block;
client.import(BlockOrigin::NetworkInitialSync, a2.clone()).unwrap();
let mut b1 = client.new_block_at(
&BlockId::Number(0),
Default::default(),
false,
).unwrap();
// needed to make sure B1 gets a different hash from A1
b1.push_transfer(Transfer {
from: AccountKeyring::Alice.into(),
to: AccountKeyring::Ferdie.into(),
amount: 1,
nonce: 0,
}).unwrap();
let b1 = b1.build().unwrap().block;
client.import(BlockOrigin::NetworkInitialSync, b1.clone()).unwrap();
let b2 = client.new_block_at(
&BlockId::Hash(b1.hash()),
Default::default(),
false,
).unwrap().build().unwrap().block;
// Should trigger a notification because we reorg
client.import_as_best(BlockOrigin::NetworkInitialSync, b2.clone()).unwrap();
// There should be one notification
let notification = notification_stream.next().unwrap();
// We should have a tree route of the re-org
let tree_route = notification.tree_route.unwrap();
assert_eq!(tree_route.enacted()[0].hash, b1.hash());
}