Use JustifiedBlockAnnounceValidator for parachain block announce validator #96
Conversation
Forked at: 032595d Parent branch: origin/master
Parent branch: origin/master Forked at: 032595d
Parent branch: origin/master Forked at: 032595d
Parent branch: origin/master Forked at: 032595d
Parent branch: origin/master Forked at: 032595d
Forked at: d6ab13c Parent branch: origin/master
Co-authored-by: Peter Goodspeed-Niklaus <[email protected]>
cecton
force-pushed
the
cecton-use-custom-block-announce-validator
branch
from
db8cab2 to
6e43fd4
Compare
| // run cumulus dave | ||
| let cumulus_dave_dir = tempdir().unwrap(); | ||
| let mut cumulus_dave = Command::new(cargo_bin("cumulus-test-parachain-collator")) | ||
| .stdout(Stdio::piped()) | ||
| .stderr(Stdio::piped()) | ||
| .arg("--base-path") | ||
| .arg(cumulus_dave_dir.path()) | ||
| .arg("--unsafe-rpc-expose") | ||
| .arg("--rpc-port=27018") | ||
| .arg("--port=27118") | ||
| .arg("--") | ||
| .arg(format!( | ||
| "--bootnodes=/ip4/127.0.0.1/tcp/27115/p2p/{}", | ||
| polkadot_alice_id | ||
| )) | ||
| .arg(format!( | ||
| "--bootnodes=/ip4/127.0.0.1/tcp/27116/p2p/{}", | ||
| polkadot_bob_id | ||
| )) | ||
| .arg("--dave") | ||
| .spawn() | ||
| .unwrap(); | ||
| let cumulus_dave_helper = ChildHelper::new("cumulus-dave", &mut cumulus_dave); | ||
| wait_for_tcp("127.0.0.1:27018").await; | ||
|
|
||
| // connect rpc client to cumulus | ||
| let transport_client_cumulus_dave = | ||
| jsonrpsee::transport::http::HttpTransportClient::new("http://127.0.0.1:27018"); | ||
| let mut client_cumulus_dave = jsonrpsee::raw::RawClient::new(transport_client_cumulus_dave); | ||
|
|
||
| // wait for parachain blocks to be produced | ||
| let number_of_blocks = 4; | ||
| let mut previous_blocks = HashSet::with_capacity(number_of_blocks); | ||
| loop { | ||
| let current_block_hash = Chain::block_hash(&mut client_cumulus_dave, None) | ||
| .await | ||
| .unwrap() | ||
| .unwrap(); | ||
|
|
||
| if previous_blocks.insert(current_block_hash) { | ||
| eprintln!("new parachain block: {}", current_block_hash); | ||
|
|
||
| if previous_blocks.len() == number_of_blocks { | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| sleep(Duration::from_secs(2)).await; | ||
| } |
There was a problem hiding this comment.
I also added a test in the integration test but it is not super useful, it will only detect something if the implementation is broken but not if the implementation returns always Success like the default implementation of BlockAnnounceValidator
cecton
force-pushed
the
cecton-use-custom-block-announce-validator
branch
from
f84d067 to
c9f95a3
Compare
|
|
||
| // run cumulus dave | ||
| let cumulus_dave_dir = tempdir().unwrap(); | ||
| let mut cumulus_dave = Command::new(cargo_bin("cumulus-test-parachain-collator")) |
There was a problem hiding this comment.
As I said in #96 (comment)
I also added a test in the integration test but it is not super useful, it will only detect something if the implementation is broken but not if the implementation returns always Success like the default implementation of BlockAnnounceValidator.
So for example if the implementation is broken and no block at all are validated, then we would see on Dave that no block are being imported and the test would fail.
| // wait for parachain blocks to be produced | ||
| let number_of_blocks = 4; | ||
| let mut previous_blocks = HashSet::with_capacity(number_of_blocks); | ||
| loop { |
There was a problem hiding this comment.
I expect that these tests are split and not that we just grow this one test. Or that we make the tests based on each other (by reusing the db or similar).
However, we should at least start to refactor stuff into functions. You just copied the same code.
There was a problem hiding this comment.
I just refactored. Should I go further or is it good for now?
| } | ||
| } | ||
|
|
||
| pub struct DelayedBlockAnnounceValidator<B: BlockT>(Arc<Mutex<Option<Box<dyn BlockAnnounceValidator<B> + Send>>>>); |
There was a problem hiding this comment.
Docs and especially why we do this! + an issue that we need to fix this.
There was a problem hiding this comment.
You should create an issue for this. And this should contain this info.
Co-authored-by: Bastian Köcher <[email protected]>
| } | ||
| } | ||
|
|
||
| pub struct DelayedBlockAnnounceValidator<B: BlockT>(Arc<Mutex<Option<Box<dyn BlockAnnounceValidator<B> + Send>>>>); |
There was a problem hiding this comment.
You should create an issue for this. And this should contain this info.
Related to #7