Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.

Commit 6f326d4

Browse files
niklasad1niklasad1
committed
Merge remote-tracking branch 'origin/dp-jsonrpsee-integration-2' into na-fix-nit
2 parents d91b15c + 1b14b0b commit 6f326d4

File tree

2 files changed

+103
-89
lines changed

2 files changed

+103
-89
lines changed

‎client/rpc/src/offchain/tests.rs‎

Lines changed: 60 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -1,55 +1,60 @@
1-
// // This file is part of Substrate.
2-
3-
// // Copyright (C) 2020-2021 Parity Technologies (UK) Ltd.
4-
// // SPDX-License-Identifier: GPL-3.0-or-later WITH Classpath-exception-2.0
5-
6-
// // This program is free software: you can redistribute it and/or modify
7-
// // it under the terms of the GNU General Public License as published by
8-
// // the Free Software Foundation, either version 3 of the License, or
9-
// // (at your option) any later version.
10-
11-
// // This program is distributed in the hope that it will be useful,
12-
// // but WITHOUT ANY WARRANTY; without even the implied warranty of
13-
// // MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
14-
// // GNU General Public License for more details.
15-
16-
// // You should have received a copy of the GNU General Public License
17-
// // along with this program. If not, see <https://www.gnu.org/licenses/>.
18-
19-
// use super::*;
20-
// use assert_matches::assert_matches;
21-
// use sp_core::{offchain::storage::InMemOffchainStorage, Bytes};
22-
23-
// #[test]
24-
// fn local_storage_should_work() {
25-
// let storage = InMemOffchainStorage::default();
26-
// let offchain = Offchain::new(storage, DenyUnsafe::No);
27-
// let key = Bytes(b"offchain_storage".to_vec());
28-
// let value = Bytes(b"offchain_value".to_vec());
29-
30-
// assert_matches!(
31-
// offchain.set_local_storage(StorageKind::PERSISTENT, key.clone(), value.clone()),
32-
// Ok(())
33-
// );
34-
// assert_matches!(
35-
// offchain.get_local_storage(StorageKind::PERSISTENT, key),
36-
// Ok(Some(ref v)) if *v == value
37-
// );
38-
// }
39-
40-
// #[test]
41-
// fn offchain_calls_considered_unsafe() {
42-
// let storage = InMemOffchainStorage::default();
43-
// let offchain = Offchain::new(storage, DenyUnsafe::Yes);
44-
// let key = Bytes(b"offchain_storage".to_vec());
45-
// let value = Bytes(b"offchain_value".to_vec());
46-
47-
// assert_matches!(
48-
// offchain.set_local_storage(StorageKind::PERSISTENT, key.clone(), value.clone()),
49-
// Err(Error::UnsafeRpcCalled(_))
50-
// );
51-
// assert_matches!(
52-
// offchain.get_local_storage(StorageKind::PERSISTENT, key),
53-
// Err(Error::UnsafeRpcCalled(_))
54-
// );
55-
// }
1+
// This file is part of Substrate.
2+
3+
// Copyright (C) 2020-2021 Parity Technologies (UK) Ltd.
4+
// SPDX-License-Identifier: GPL-3.0-or-later WITH Classpath-exception-2.0
5+
6+
// This program is free software: you can redistribute it and/or modify
7+
// it under the terms of the GNU General Public License as published by
8+
// the Free Software Foundation, either version 3 of the License, or
9+
// (at your option) any later version.
10+
11+
// This program is distributed in the hope that it will be useful,
12+
// but WITHOUT ANY WARRANTY; without even the implied warranty of
13+
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
14+
// GNU General Public License for more details.
15+
16+
// You should have received a copy of the GNU General Public License
17+
// along with this program. If not, see <https://www.gnu.org/licenses/>.
18+
19+
use super::*;
20+
use assert_matches::assert_matches;
21+
use sp_core::{offchain::storage::InMemOffchainStorage, Bytes};
22+
23+
#[test]
24+
fn local_storage_should_work() {
25+
let storage = InMemOffchainStorage::default();
26+
let offchain = Offchain::new(storage, DenyUnsafe::No);
27+
let key = Bytes(b"offchain_storage".to_vec());
28+
let value = Bytes(b"offchain_value".to_vec());
29+
30+
assert_matches!(
31+
offchain.set_local_storage(StorageKind::PERSISTENT, key.clone(), value.clone()),
32+
Ok(())
33+
);
34+
assert_matches!(
35+
offchain.get_local_storage(StorageKind::PERSISTENT, key),
36+
Ok(Some(ref v)) if *v == value
37+
);
38+
}
39+
40+
#[test]
41+
fn offchain_calls_considered_unsafe() {
42+
use jsonrpsee::types::CallError;
43+
let storage = InMemOffchainStorage::default();
44+
let offchain = Offchain::new(storage, DenyUnsafe::Yes);
45+
let key = Bytes(b"offchain_storage".to_vec());
46+
let value = Bytes(b"offchain_value".to_vec());
47+
48+
assert_matches!(
49+
offchain.set_local_storage(StorageKind::PERSISTENT, key.clone(), value.clone()),
50+
Err(JsonRpseeError::Call(CallError::Failed(err))) => {
51+
assert_eq!(err.to_string(), "RPC call is unsafe to be called externally")
52+
}
53+
);
54+
assert_matches!(
55+
offchain.get_local_storage(StorageKind::PERSISTENT, key),
56+
Err(JsonRpseeError::Call(CallError::Failed(err))) => {
57+
assert_eq!(err.to_string(), "RPC call is unsafe to be called externally")
58+
}
59+
);
60+
}

‎client/rpc/src/state/tests.rs‎

Lines changed: 43 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -266,13 +266,13 @@ async fn should_notify_about_storage_changes() {
266266
};
267267

268268
// We should get a message back on our subscription about the storage change:
269-
// TODO (jsdw): previously we got back 2 messages here.
270-
// TODO (dp): I agree that we differ here. I think `master` always includes the initial value of
271-
// the storage?
269+
// NOTE: previous versions of the subscription code used to return an empty value for the
270+
// "initial" storage change here
272271
let msg = timeout_secs(1, sub_rx.next()).await;
273272
assert_matches!(&msg, Ok(Some(json)) => {
274273
serde_json::from_str::<SubscriptionResponse<StorageChangeSet<H256>>>(&json).expect("The right kind of response")
275274
});
275+
276276
let err = timeout_secs(1, sub_rx.next()).await;
277277
assert_matches!(&err, Ok(Some(json)) => {
278278
serde_json::from_str::<SubscriptionResponse<SubscriptionClosedError>>(&json).expect("The right kind of response")
@@ -281,39 +281,52 @@ async fn should_notify_about_storage_changes() {
281281

282282
#[tokio::test]
283283
async fn should_send_initial_storage_changes_and_notifications() {
284-
let mut client = Arc::new(substrate_test_runtime_client::new());
285-
let (api, _child) =
286-
new_full(client.clone(), SubscriptionTaskExecutor::new(TaskExecutor), DenyUnsafe::No, None);
284+
let mut sub_rx = {
285+
let mut client = Arc::new(substrate_test_runtime_client::new());
286+
let (api, _child) = new_full(
287+
client.clone(),
288+
SubscriptionTaskExecutor::new(TaskExecutor),
289+
DenyUnsafe::No,
290+
None,
291+
);
287292

288-
let alice_balance_key =
289-
blake2_256(&runtime::system::balance_of_key(AccountKeyring::Alice.into()));
293+
let alice_balance_key =
294+
blake2_256(&runtime::system::balance_of_key(AccountKeyring::Alice.into()));
290295

291-
let api_rpc = api.into_rpc();
292-
let (_sub_id, mut sub_rx) = api_rpc
293-
.test_subscription(
294-
"state_subscribeStorage",
295-
Some(to_raw_value(&[StorageKey(alice_balance_key.to_vec())]).unwrap()),
296-
)
297-
.await;
298-
299-
let mut builder = client.new_block(Default::default()).unwrap();
300-
builder
301-
.push_transfer(runtime::Transfer {
302-
from: AccountKeyring::Alice.into(),
303-
to: AccountKeyring::Ferdie.into(),
304-
amount: 42,
305-
nonce: 0,
306-
})
307-
.unwrap();
308-
let block = builder.build().unwrap().block;
309-
client.import(BlockOrigin::Own, block).await.unwrap();
296+
let api_rpc = api.into_rpc();
297+
let (_sub_id, sub_rx) = api_rpc
298+
.test_subscription(
299+
"state_subscribeStorage",
300+
Some(to_raw_value(&[vec![StorageKey(alice_balance_key.to_vec())]]).unwrap()),
301+
)
302+
.await;
303+
304+
let mut builder = client.new_block(Default::default()).unwrap();
305+
builder
306+
.push_transfer(runtime::Transfer {
307+
from: AccountKeyring::Alice.into(),
308+
to: AccountKeyring::Ferdie.into(),
309+
amount: 42,
310+
nonce: 0,
311+
})
312+
.unwrap();
313+
let block = builder.build().unwrap().block;
314+
client.import(BlockOrigin::Own, block).await.unwrap();
315+
316+
sub_rx
317+
};
310318

311319
// Check for the correct number of notifications
312320
let msgs = timeout_secs(5, (&mut sub_rx).take(2).collect::<Vec<_>>()).await;
313-
assert_matches!(msgs, Ok(_));
321+
assert_matches!(&msgs, Ok(json_vals) => {
322+
let vals: Vec<_> = json_vals.iter().map(|json| serde_json::from_str::<SubscriptionResponse<StorageChangeSet<H256>>>(&json).expect("The right kind of response")).collect();
323+
});
314324

315325
// No more messages to follow
316-
assert_matches!(timeout_secs(1, sub_rx.next()).await, Ok(None));
326+
let err = timeout_secs(1, sub_rx.next()).await;
327+
assert_matches!(&err, Ok(Some(json)) => {
328+
serde_json::from_str::<SubscriptionResponse<SubscriptionClosedError>>(&json).expect("The right kind of response")
329+
});
317330
}
318331

319332
#[tokio::test]
@@ -573,11 +586,7 @@ async fn should_notify_on_runtime_version_initially() {
573586
// assert initial version sent.
574587
assert_matches!(timeout_secs(1, sub_rx.next()).await, Ok(Some(_)));
575588

576-
// TODO (jsdw): The channel remains open here, so waiting for another message will time out.
577-
// Previously the channel returned None.
578-
// TODO (dp): I think this is a valid concern; our version swallows the `None` (in the
579-
// `take_while` call I guess?). I guess this test does what is says on the tin though: check
580-
// that we get the current runtime version when subscribing.
589+
// TODO(niklasad1): make sure we get subscription closed here.
581590
assert_matches!(timeout_secs(1, sub_rx.next()).await, Err(_));
582591
}
583592

0 commit comments

Comments
 (0)