Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
75 commits
Select commit Hold shift + click to select a range
2f5367f
Rough skeleton for what I think the RPC should look like
HCastano Apr 8, 2020
9d369ac
Create channel for sending justifications
HCastano Apr 10, 2020
0095b78
WIP: Add subscribers for justifications to Grandpa
HCastano Apr 18, 2020
7719292
WIP: Add a struct for managing subscriptions
HCastano Apr 21, 2020
8c35b1e
Make naming more clear and lock data in Arc
HCastano Apr 21, 2020
07ef5fa
Rough idea of what RPC would look like
HCastano Apr 21, 2020
c9aa9e2
Remove code from previous approach
HCastano Apr 22, 2020
78250b7
Missed some things
HCastano Apr 22, 2020
fe31500
Update client/rpc-api/src/chain/mod.rs
HCastano Apr 22, 2020
ba13ee6
Update client/rpc-api/src/chain/mod.rs
HCastano Apr 22, 2020
fe360b9
Split justification subscription into sender and receiver halves
HCastano Apr 26, 2020
254bebe
Replace RwLock with a Mutex
HCastano Apr 26, 2020
9c996ea
Add sample usage from the Service's point of view
HCastano Apr 26, 2020
8090161
Merge branch 'master' into hc-add-subscription-rpc-for-finality
HCastano Apr 29, 2020
1150e5d
Merge branch 'master' into hc-add-subscription-rpc-for-finality
HCastano May 7, 2020
130a871
Merge branch 'master' into hc-add-subscription-rpc-for-finality
HCastano May 15, 2020
f42b6c3
Remove code that referred to "chain_" RPC
HCastano May 15, 2020
d61d7ec
Use the Justification sender/receivers from Grandpa LinkHalf
HCastano May 17, 2020
56e716b
Add some PubSub boilerplate
HCastano May 18, 2020
c0c6508
Merge branch 'master' into hc-add-subscription-rpc-for-finality
HCastano May 18, 2020
1bc9103
Merge branch 'master' into hc-add-subscription-rpc-for-finality
HCastano Jun 1, 2020
30e3831
Merge branch 'master' into hc-add-subscription-rpc-for-finality
HCastano Jun 3, 2020
8b0850a
Add guiding comments
HCastano Jun 3, 2020
cf07f2f
TMP: comment out to fix compilation
octol Jun 8, 2020
ab64ecb
Merge branch 'master' into hc-add-subscription-rpc-for-finality
octol Jun 9, 2020
b1c04ba
Return MetaIoHandler from PubSubHandler in create_full
octol Jun 10, 2020
ebad4d7
Uncomment pubsub methods in rpc handler (fails to build)
octol Jun 10, 2020
caaaa61
node/rpc: make Metadata concrete in create_full to fix compilation
octol Jun 11, 2020
f3cc272
node: pass in SubscriptionManger to grandpa rpc handler
octol Jun 11, 2020
3c4505a
grandpa-rpc: use SubscriptionManger to add subscriber
octol Jun 12, 2020
9a225ac
grandpa-rpc: attempt at setting up the justification stream (fails to…
octol Jun 16, 2020
a85dd53
grandpa-rpc: fix compilation of connecting stream to sink
octol Jun 16, 2020
1b0344f
grandpa-rpc: implement unsubscribe
octol Jun 16, 2020
12dd5df
grandpa-rpc: update older tests
octol Jun 18, 2020
bca6a13
grandpa-rpc: add full prefix to avoid confusing rust-analyzer
octol Jun 22, 2020
efaa0d1
grandpa-rpc: add test for pubsub not available
octol Jun 22, 2020
9632988
grandpa-rpc: tidy up leftover code
octol Jun 22, 2020
964f1e3
grandpa-rpc: add test for sub and unsub of justifications
octol Jun 22, 2020
82004cb
grandpa-rpc: minor stylistic changes
octol Jun 23, 2020
e526f95
grandpa-rpc: split unit test
octol Jun 23, 2020
6c71b8f
grandpa-rpc: minor stylistic changes in test
octol Jun 23, 2020
8bfb560
grandpa-rpc: skip returning future when cancelling
octol Jun 25, 2020
be7cb78
grandpa-rpc: reuse testing executor from sc-rpc
octol Jun 25, 2020
ac81cdf
grandpa-rpc: don't need to use PubSubHandler in tests
octol Jun 25, 2020
3f8630d
node-rpc: use MetaIoHandler rather than PubSubHandler
octol Jun 26, 2020
3b387e6
grandpa: log if getting header failed
octol Jun 26, 2020
bf4b19a
grandpa: move justification channel creation into factory function
octol Jun 26, 2020
4c96b73
grandpa: make the justification sender optional
octol Jun 26, 2020
12158a2
grandpa: fix compilation warnings
octol Jun 27, 2020
e2584e5
grandpa: move justification notification types to new file
octol Jun 27, 2020
dc3c888
grandpa-rpc: move JustificationNotification to grandpa-rpc
octol Jun 27, 2020
410af1f
grandpa-rpc: move JustificationNotification to its own file
octol Jun 27, 2020
fb3ad63
grandpa: rename justification channel pairs
octol Jun 27, 2020
5d80399
grandpa: rename notifier types
octol Jun 27, 2020
3f33fd2
grandpa: pass justification as GrandpaJustification to the rpc module
octol Jun 29, 2020
57f1963
Merge branch 'master' into hc-add-subscription-rpc-for-finality
octol Jun 29, 2020
90ae56e
Move Metadata to sc-rpc-api
tomusdrw Jun 30, 2020
be1df4f
grandpa-rpc: remove unsed error code
octol Jun 29, 2020
d4cddf3
grandpa: fix bug for checking if channel is closed before sendind
octol Jul 1, 2020
8bd9e5c
grandpa-rpc: unit test for sending justifications
octol Jun 30, 2020
127c19e
Merge branch 'master' into hc-add-subscription-rpc-for-finality
octol Jul 2, 2020
f43ee9b
grandpa-rpc: update comments for the pubsub test
octol Jul 2, 2020
39640c1
grandpa-rpc: update pubsub tests with more steps
octol Jul 2, 2020
f3c6c54
grandpa-rpc: fix pubsub test
octol Jul 2, 2020
44ba73e
grandpa-rpc: minor indendation
octol Jul 3, 2020
01f7d73
Merge branch 'master' into hc-add-subscription-rpc-for-finality
octol Jul 22, 2020
3f3b136
grandpa-rpc: decode instead of encode in test
octol Jul 23, 2020
908cf15
Merge branch 'master' into hc-add-subscription-rpc-for-finality
octol Jul 27, 2020
bb9ad4b
Merge branch 'master' into hc-add-subscription-rpc-for-finality
octol Jul 27, 2020
410ddea
grandpa: fix review comments
octol Jul 31, 2020
ca126fd
Merge branch 'master' into hc-add-subscription-rpc-for-finality
octol Aug 4, 2020
452024f
grandpa: remove unused serde dependency
octol Aug 4, 2020
8b0cf76
Merge branch 'master' into hc-add-subscription-rpc-for-finality
octol Aug 5, 2020
7ddffef
Merge branch 'master' into hc-add-subscription-rpc-for-finality
octol Aug 6, 2020
817b425
Merge remote-tracking branch 'upstream/master' into hc-add-subscripti…
octol Aug 7, 2020
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
grandpa: pass justification as GrandpaJustification to the rpc module
  • Loading branch information
octol committed Jun 29, 2020
commit 3f33fd2c83f2cec57e8c82a54e3334d033a47a1f
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions client/finality-grandpa/rpc/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ serde_json = "1.0.50"
log = "0.4.8"
derive_more = "0.99.2"
parking_lot = "0.10.0"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused?

parity-scale-codec = { version = "1.3.0", features = ["derive"] }

[dev-dependencies]
sp-core = { version = "2.0.0-rc3", path = "../../../primitives/core" }
Expand Down
8 changes: 5 additions & 3 deletions client/finality-grandpa/rpc/src/notification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@
// along with this program. If not, see <https://www.gnu.org/licenses/>.

use serde::{Serialize, Deserialize};
use parity_scale_codec::Encode;
use sp_runtime::traits::Block as BlockT;
use sc_finality_grandpa::GrandpaJustification;

/// Justification for a finalized block.
#[derive(Clone, Serialize, Deserialize)]
Expand All @@ -28,11 +30,11 @@ pub struct JustificationNotification<Block: BlockT> {
pub justification: Vec<u8>,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason for doing this is so that we don't need to implement Serialize/Deserialize for GrandpaJustification? Not entirely sure whether it makes sense to dump a blob on the users of the API.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, in particular finality_grandpa::Commit doesn't either implement Serialize/Deserialize

}

impl<Block: BlockT> From<(Block::Header, Vec<u8>)> for JustificationNotification<Block> {
fn from(notification: (Block::Header, Vec<u8>)) -> Self {
impl<Block: BlockT> From<(Block::Header, GrandpaJustification<Block>)> for JustificationNotification<Block> {
fn from(notification: (Block::Header, GrandpaJustification<Block>)) -> Self {
JustificationNotification {
header: notification.0,
justification: notification.1,
justification: notification.1.encode(),
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
}
}

11 changes: 7 additions & 4 deletions client/finality-grandpa/src/environment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1148,7 +1148,7 @@ pub(crate) fn finalize_block<BE, Block, Client>(
// justifications for transition blocks which will be requested by
// syncing clients.
let justification = match justification_or_commit {
JustificationOrCommit::Justification(justification) => Some(justification.encode()),
JustificationOrCommit::Justification(justification) => Some(justification),
JustificationOrCommit::Commit((round_number, commit)) => {
let mut justification_required =
// justification is always required when block that enacts new authorities
Expand All @@ -1175,17 +1175,18 @@ pub(crate) fn finalize_block<BE, Block, Client>(
commit,
)?;

Some(justification.encode())
Some(justification)
} else {
None
}
},
};

if let Some(justification) = justification.clone() {
// Notify any registered listeners in case we have a justification
if let Some(ref justification) = justification {
match client.header(BlockId::Hash(hash)) {
Ok(Some(header)) => {
let notification = (header, justification);
let notification = (header, justification.clone());
if let Some(sender) = justification_sender {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be checked earlier, so that we don't even request client.header if justification_sender.is_none().

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have a strict opinion on this but it could also be done from the RPC side. You only need to send the justification to the RPC side and it can then fetch the header for the given finalized hash (justification.commit.target_hash). There's also the possibility that we don't need the header at all as I commented somewhere else :P

let _ = sender.notify(notification);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be the correct plate to notify, but would be good if the reviewers could give this part extra attention, thanks :)

}
Expand All @@ -1195,6 +1196,8 @@ pub(crate) fn finalize_block<BE, Block, Client>(
};
}

let justification = justification.map(|j| j.encode());

debug!(target: "afg", "Finalizing blocks up to ({:?}, {})", number, hash);

// ideally some handle to a synchronization oracle would be used
Expand Down
2 changes: 1 addition & 1 deletion client/finality-grandpa/src/justification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ use crate::{Commit, Error};
///
/// This is meant to be stored in the db and passed around the network to other
/// nodes, and are used by syncing nodes to prove authority set handoffs.
#[derive(Encode, Decode)]
#[derive(Clone, Encode, Decode)]
pub struct GrandpaJustification<Block: BlockT> {
round: u64,
pub(crate) commit: Commit<Block>,
Expand Down
4 changes: 3 additions & 1 deletion client/finality-grandpa/src/notification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,10 @@ use parking_lot::Mutex;
use sp_runtime::traits::Block as BlockT;
use sp_utils::mpsc::{tracing_unbounded, TracingUnboundedSender, TracingUnboundedReceiver};

use crate::justification::GrandpaJustification;

/// Justification for a finalized block.
type JustificationNotification<Block> = (<Block as BlockT>::Header, Vec<u8>);
type JustificationNotification<Block> = (<Block as BlockT>::Header, GrandpaJustification<Block>);

// Stream of justifications returned when subscribing.
type JustificationStream<Block> = TracingUnboundedReceiver<JustificationNotification<Block>>;
Expand Down