zebra icon indicating copy to clipboard operation
zebra copied to clipboard

Add a `getblocktemplate-rpcs` feature to `zebrad` and `zebra-rpc`

Open mpguerra opened this issue 2 years ago • 5 comments

mpguerra avatar Sep 29 '22 09:09 mpguerra

It seems we have a few options here, not sure if they are written somewhere else.

It seems we don't need this feature in zebrad at least until we make a call to one of the new methods from an acceptance test.

We do need it in zebra-rpc so we can add new methods.

The Rpc trait implements all the methods so we can:

  • Add #[cfg(feature = "getblocktemplate-rpcs")] on top of each function we declare, on top of each function implementation and on top of each test that will use any of this new code. Or
  • We can add it once in the methods.rs file like
     #[cfg(feature = "getblocktemplate-rpcs")]
    pub mod getblocktemplate;
    

Adding it once and have all the code in a mod seems to be a bit better however i am not sure if we can easily add new methods to the base trait.

oxarbitrage avatar Oct 05 '22 18:10 oxarbitrage

It seems we don't need this feature in zebrad at least until we make a call to one of the new methods from an acceptance test.

We've added acceptance tests to each RPC ticket, so we'll probably want that feature at the same time. (It's quick to add a new feature.)

We do need it in zebra-rpc so we can add new methods.

The Rpc trait implements all the methods so we can:

* Add `#[cfg(feature = "getblocktemplate-rpcs")]` on top of each function we declare, on top of each function implementation and on top of each test that will use any of this new code. Or

* We can add it once in the methods.rs file like
  ```
   #[cfg(feature = "getblocktemplate-rpcs")]
  pub mod getblocktemplate;
  ```

Adding it once and have all the code in a mod seems to be a bit better however i am not sure if we can easily add new methods to the base trait.

We could add a getblocktemplate module, with a:

  • GetBlockTemplateRpc supertrait, and
  • an impl GetBlockTemplateRpc for Rpc block?

https://doc.rust-lang.org/rust-by-example/trait/supertraits.html

Then if the feature is active, we automatically get the new trait impl, and if it's not, we don't. (We'll have to check if this works with #[rpc(server)].)

teor2345 avatar Oct 05 '22 23:10 teor2345

Hey team! Please add your planning poker estimate with Zenhub @arya2 @conradoplg @dconnolly @oxarbitrage @teor2345 @upbqdn

mpguerra avatar Oct 06 '22 09:10 mpguerra

we need to implement at least one rpc call before we can "hide it" behind a feature

mpguerra avatar Oct 11 '22 12:10 mpguerra

we need to implement at least one rpc call before we can "hide it" behind a feature

The PR https://github.com/ZcashFoundation/zebra/pull/5357 attempt to implement the getblockcount and the feature. getblockcount was selected as it is really easy an one. (just return the current chain tip height)

I think we should go that way (implementing the feature first) to avoid the refactor (moving code among files) that we will have to do if we use the supertrait design to implemnent this.

If we add the feature function by function then we add the feature code with each call but initially we want to try the other way as it is somehow cleaner.

oxarbitrage avatar Oct 11 '22 13:10 oxarbitrage