Expand chain api #538
Expand chain api #538
Conversation
Niederb
added
F8-newfeature
examples/examples/get_blocks.rs
Outdated
|
|
||
| let header_hash = api.get_finalized_head().unwrap().unwrap(); | ||
| println!("Latest Finalized Header Hash:\n {} \n", header_hash); | ||
|
|
||
| let h = api.get_header(Some(header_hash)).unwrap().unwrap(); | ||
| println!("Finalized header:\n {:?} \n", h); | ||
|
|
||
| let b = api.get_signed_block(Some(header_hash)).unwrap().unwrap(); | ||
| println!("Finalized signed block:\n {:?} \n", b); | ||
| let b = api.get_finalized_block().unwrap().unwrap(); |
There was a problem hiding this comment.
| let b = api.get_finalized_block().unwrap().unwrap(); | |
| let signed_block = api.get_finalized_block().unwrap().unwrap(); |
examples/examples/get_blocks.rs
Outdated
| let b = api.get_finalized_block().unwrap().unwrap(); | ||
| println!("Finalized block:\n {:?} \n", b); | ||
| let last_block_number = b.block.header.number; | ||
| let block_numbers = std::cmp::max(0, last_block_number - 3)..last_block_number; |
There was a problem hiding this comment.
I think this part is a little too complex for an example. It needs quite some thinking on what exactly is done here.
How about adding comments or use different variable names? Such as number_of_last_three_blocks = ...
Would it be possible to add the .collect to the first line? E.g.
number_of_last_three_blocks: Vec<BlockNumber> = (std::cmp::max(0, last_block_number - 3)..last_block_number).collect()There was a problem hiding this comment.
I made it a bit clearer and added some comments
| @@ -76,6 +80,13 @@ pub trait GetBlock { | |||
| &self, | |||
| number: Option<Self::BlockNumber>, | |||
| ) -> Result<Option<SignedBlock<Self::Block>>>; | |||
|
|
|||
| fn get_finalized_block(&self) -> Result<Option<SignedBlock<Self::Block>>>; | |||
There was a problem hiding this comment.
Could you add some comments? E.g. /// Get the last finalized signed block.
| @@ -116,6 +132,25 @@ where | |||
| ) -> Result<Option<SignedBlock<Self::Block>>> { | |||
| self.get_block_hash(number).map(|h| self.get_signed_block(h))? | |||
| } | |||
|
|
|||
| fn get_finalized_block(&self) -> Result<Option<SignedBlock<Self::Block>>> { | |||
| self.get_finalized_head()? | |||
There was a problem hiding this comment.
With this, two prior independent traits are now not independent anymore. Since GetHeader is so small anyway, it might be sensible to merge these two traits to a GetChain Trait.
What do you think?
There was a problem hiding this comment.
Yes, I think merging the traits makes sense.
There was a problem hiding this comment.
I merged the two traits now into the new GetChainInfo trait
| @@ -116,6 +132,25 @@ where | |||
| ) -> Result<Option<SignedBlock<Self::Block>>> { | |||
| self.get_block_hash(number).map(|h| self.get_signed_block(h))? | |||
| } | |||
|
|
|||
| fn get_finalized_block(&self) -> Result<Option<SignedBlock<Self::Block>>> { | |||
| self.get_finalized_head()? | |||
get_finalized_head()inside ofget_finalized_block()I had to add a trait bound forRuntime::Headerbut I think it is not too limiting.genesis_hash()inAPI. I implementedget_genesis_block()instead.GetBlockandGetHeadertraits into a singleGetChainInfotrait