feat: add frigate electrum based RPC methods - #16
Conversation
server.version, blockchain.silentpayments.subscribe and blockchain.silentpayments.unsubscribe225fc0c to
885cd2f
Compare
|
Thanks for the PR! Note that it needs a rebase now with the recent merges. |
… `blockchain.silentpayments.unsubscribe`
oleonardolima
left a comment
There was a problem hiding this comment.
In https://github.com/sparrowwallet/frigate#serverfeatures it mentions the new field in server.features, is it not needed by bdk-sp ?
| /// A request to establish connection with Frigate Electrum client | ||
| /// | ||
| /// This corresponds to the `"server.version"` Frigate Electrum RPC method | ||
| /// | ||
| /// See: https://github.com/sparrowwallet/frigate | ||
| #[cfg(feature = "frigate")] | ||
| #[derive(Debug, Clone, PartialEq, Eq, Hash)] | ||
| pub struct Version { | ||
| pub client_name: CowStr, | ||
| pub version: CowStr, | ||
| } | ||
|
|
||
| #[cfg(feature = "frigate")] | ||
| impl Request for Version { | ||
| type Response = Vec<String>; | ||
|
|
||
| fn to_method_and_params(&self) -> MethodAndParams { | ||
| ( | ||
| "server.version".into(), | ||
| vec![self.client_name.clone().into(), self.version.clone().into()], | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
The server.version is not an exclusive method of frigate, so I don't think the feature is needed here. Also, if it lands after #11, it'll already be covered.
There was a problem hiding this comment.
Yes indeed, we just need to coordinate which lands first so that I can remove it here.
| pub struct SpSubscribe { | ||
| pub scan_priv_key: bitcoin::secp256k1::SecretKey, | ||
| pub scan_pub_key: bitcoin::secp256k1::PublicKey, | ||
| pub start_height: Option<u32>, |
There was a problem hiding this comment.
In https://github.com/sparrowwallet/frigate#blockchainsilentpaymentssubscribe it also mentions that start_height could be a string in the form FROM-TO, is that format intended to be ignored ?
There was a problem hiding this comment.
It's true it's not mentioned anywhere here but the initial code was for version 1.3.2 (https://github.com/sparrowwallet/frigate/tree/5124333b145acc2a6385f5cd4aeb918c8c896cd7#blockchainsilentpaymentssubscribe)
There has been quite some updates in the meantime. I'm just realising too :)
|
@sdmg15 I just thought this now during the call, but you could also try adding support for these in https://github.com/bitcoindevkit/rust-electrum-client, not sure if the architecture there supports it though. |
| /// Corresponds to `"blockchain.silentpayments.subscribe"` Frigate Electrum notification method | ||
| #[cfg(feature = "frigate")] | ||
| #[derive(Debug, Clone, serde::Deserialize)] | ||
| pub struct SpNotification { |
There was a problem hiding this comment.
I think it's good for consistency to use the same pattern as the other notifications.
pub struct SpNotification {
param_0: SpSubscription,
param_1: f32,
param_2: Vec<TxTweak>,
}
#[cfg(feature = "frigate")]
impl SpNotification {
/// Returns the subscription this notification belongs to.
pub fn subscription(&self) -> &SpSubscription {
&self.param_0
}
/// Returns the scan progress, where `1.0` means up to date.
pub fn progress(&self) -> f32 {
self.param_1
}
/// Returns the transactions discovered by this notification.
pub fn history(&self) -> &[TxTweak] {
&self.param_2
}
}|
|
||
| #[cfg(feature = "frigate")] | ||
| #[derive(Debug, Clone, serde::Deserialize)] | ||
| pub struct SpSubscription { |
There was a problem hiding this comment.
Let's rename it to SpSubscribeResp for consistency and move to src/response.rs
|
|
||
| #[cfg(feature = "frigate")] | ||
| impl Request for SpSubscribe { | ||
| type Response = String; |
There was a problem hiding this comment.
This should be SpSubscription (see here)
There was a problem hiding this comment.
Sorry for not mentioning this earlier but the implemented version here 1.3.2
I just updated the PR description to be clear on that.
There was a problem hiding this comment.
Ah I see. I'll have another look then :)
noahjoeris
left a comment
There was a problem hiding this comment.
Thanks for updating @sdmg15 !
I left a few more comments
| /// A request to subscribe to payment outputs belonging to the provided keys | ||
| /// | ||
| /// This corresponds to the `"blockchain.silentpayments.subscribe"` Frigate Electrum RPC method. | ||
| /// It returns The silent payment address that has been subscribed. | ||
| /// | ||
| /// See: https://github.com/sparrowwallet/frigate#blockchainsilentpaymentssubscribe |
There was a problem hiding this comment.
Because we don't add the latest version let's specify what's supported here in the docs.
I think we support up to v1.4.1 so we might add something like: Supported Frigate: ≤ 1.4.1
There was a problem hiding this comment.
I think I'd be better to put the link to the referenced tag like: https://github.com/sparrowwallet/frigate/tree/1.4.1#blockchainsilentpaymentssubscribe Wdyt?
| #[cfg(feature = "frigate")] | ||
| gen_pending_request_types! { | ||
| Header, | ||
| HeaderWithProof, |
There was a problem hiding this comment.
Let's avoid having 2 feature lists by updating the macro to support cfg feature
So we can add:
#[cfg(feature = "frigate")] Version,
#[cfg(feature = "frigate")] SpSubscribe,
#[cfg(feature = "frigate")] SpUnSubscribe,Macro:
macro_rules! gen_pending_request_types {
($($(#[$attr:meta])* $name:ident),* $(,)?) => {
/// A successfully handled request and its decoded server response.
///
/// This enum is returned when a request has been fully processed and the server replied
/// with a valid `result`. It contains both the original request and the corresponding
/// response.
///
/// `CompletedRequest` is used by the [`Event::Response`] variant to expose typed
/// request-response pairs to the caller.
///
/// You typically don't construct this manually — it is created internally by the client
/// after decoding JSON-RPC responses.
///
/// [`Event::Response`]: crate::Event::Response
#[derive(Debug, Clone)]
pub enum CompletedRequest {
$(
$(#[$attr])*
$name {
req: crate::request::$name,
resp: <crate::request::$name as Request>::Response,
},
)*
}
/// A request that received an error response from the Electrum server.
///
/// This enum represents a completed request where the server returned a JSON-RPC error
/// instead of a `result`. It contains both the original request and the associated error.
///
/// This is used by the [`Event::ResponseError`] variant to expose server-side failures
/// in a typed manner.
///
/// Like [`CompletedRequest`], this is created internally by the client during response
/// processing.
///
/// [`Event::ResponseError`]: crate::Event::ResponseError
#[derive(Debug, Clone)]
pub enum FailedRequest {
$(
$(#[$attr])*
$name {
req: crate::request::$name,
error: ResponseError,
},
)*
}
impl core::fmt::Display for FailedRequest {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
$(
$(#[$attr])*
Self::$name { req, error } => {
write!(f, "Server responsed to {:?} with error: {}", req, error)
}
)*
}
}
}
impl std::error::Error for FailedRequest {}
$(
$(#[$attr])*
impl RequestExt for crate::request::$name {
fn into_completed(self, resp: <Self as Request>::Response) -> CompletedRequest {
CompletedRequest::$name { req: self, resp }
}
fn into_failed(self, error: ResponseError) -> FailedRequest {
FailedRequest::$name { req: self, error }
}
}
)*
};
}|
@noahjoeris Thanks for the reviews. I've applied the changes. |
This PR adds supports for additional RPC methods provided by Frigate electrum based RPC server.
The added methods are:
server.version: This is the first message sent to establish connection with serverblockchain.silentpayments.subscribe: This takes a spend public key and a scan private key and return outputs belonging to the them.blockchain.silentpayments.unsubscribe: This takes a spend public key and a scan private key and unsubscribe from notifications.Some context:
This is useful for the PR opened at bitcoindevkit/bdk-sp#48 which is doing integration of frigate ephemeral scanning.
Opening this PR in order to receive feedback.
Reference:
https://github.com/sparrowwallet/frigate/
Supported Frigate version 1.3.2