[DRAFT] statement-store: re-defining RPC API surface

prdoc/pr_10421.prdoc

@@ -0,0 +1,22 @@
+title: 'statement-store: New RPC result types'
+doc:
+- audience: Node Dev
+  description: |-
+    Moved submission failures from JSON-RPC errors into structured result types:
+    - Internal submission result type changed to hold more information for clients.
+    - The "statement_submit" method now returns enum with clear status variants (New, Known, Invalid, Rejected).
+    - NetworkPriority removed as we never used it.
+    - Updated and simplified the reputation system.
+    - Runtime API wasn't changed.
+
+crates:
+- name: sc-rpc-api
+  bump: major
+- name: sc-rpc
+  bump: major
+- name: sc-network-statement
+  bump: major
+- name: sc-statement-store
+  bump: major
+- name: sp-statement-store
+  bump: major

substrate/client/network/statement/src/lib.rs

@@ -49,9 +49,7 @@ use sc_network_common::role::ObservedRole;
 use sc_network_sync::{SyncEvent, SyncEventStream};
 use sc_network_types::PeerId;
 use sp_runtime::traits::Block as BlockT;
-use sp_statement_store::{
-	Hash, NetworkPriority, Statement, StatementSource, StatementStore, SubmitResult,
-};
+use sp_statement_store::{Hash, Statement, StatementSource, StatementStore, SubmitResult};
 use std::{
 	collections::{hash_map::Entry, HashMap, HashSet},
 	iter,
@@ -78,13 +76,11 @@ mod rep {
 	/// Reputation change when a peer sends us any statement that is not invalid.
 	pub const ANY_STATEMENT_REFUND: Rep = Rep::new(1 << 4, "Any statement (refund)");
 	/// Reputation change when a peer sends us an statement that we didn't know about.
-	pub const GOOD_STATEMENT: Rep = Rep::new(1 << 7, "Good statement");
-	/// Reputation change when a peer sends us a bad statement.
-	pub const BAD_STATEMENT: Rep = Rep::new(-(1 << 12), "Bad statement");
+	pub const GOOD_STATEMENT: Rep = Rep::new(1 << 8, "Good statement");
+	/// Reputation change when a peer sends us an invalid statement.
+	pub const INVALID_STATEMENT: Rep = Rep::new(-(1 << 12), "Invalid statement");
 	/// Reputation change when a peer sends us a duplicate statement.
 	pub const DUPLICATE_STATEMENT: Rep = Rep::new(-(1 << 7), "Duplicate statement");
-	/// Reputation change when a peer sends us particularly useful statement
-	pub const EXCELLENT_STATEMENT: Rep = Rep::new(1 << 8, "High priority statement");
 }
 
 const LOG_TARGET: &str = "statement-gossip";
@@ -503,14 +499,11 @@ where
 
 	fn on_handle_statement_import(&mut self, who: PeerId, import: &SubmitResult) {
 		match import {
-			SubmitResult::New(NetworkPriority::High) =>
-				self.network.report_peer(who, rep::EXCELLENT_STATEMENT),
-			SubmitResult::New(NetworkPriority::Low) =>
-				self.network.report_peer(who, rep::GOOD_STATEMENT),
+			SubmitResult::New => self.network.report_peer(who, rep::GOOD_STATEMENT),
 			SubmitResult::Known => self.network.report_peer(who, rep::ANY_STATEMENT_REFUND),
 			SubmitResult::KnownExpired => {},
-			SubmitResult::Ignored => {},
-			SubmitResult::Bad(_) => self.network.report_peer(who, rep::BAD_STATEMENT),
+			SubmitResult::Rejected(_) => {},
+			SubmitResult::Invalid(_) => self.network.report_peer(who, rep::INVALID_STATEMENT),
 			SubmitResult::InternalError(_) => {},
 		}
 	}

substrate/client/rpc-api/Cargo.toml

@@ -27,5 +27,6 @@ serde_json = { workspace = true, default-features = true }
 sp-core = { workspace = true, default-features = true }
 sp-rpc = { workspace = true, default-features = true }
 sp-runtime = { workspace = true, default-features = true }
+sp-statement-store = { features = ["serde"], workspace = true, default-features = true }
 sp-version = { workspace = true, default-features = true }
 thiserror = { workspace = true }

substrate/client/rpc-api/src/statement/mod.rs

@@ -19,12 +19,141 @@
 //! Substrate Statement Store RPC API.
 
 use jsonrpsee::{core::RpcResult, proc_macros::rpc};
+use serde::{Deserialize, Serialize};
 use sp_core::Bytes;
 
 pub mod error;
 
+// Re-export types from primitives with serde support
+pub use sp_statement_store::SubmitResult;
+
+/// Filter for querying statements with different topics.
+#[derive(Debug, Clone, Serialize, Deserialize)]
+#[serde(rename_all = "camelCase")]
+pub enum TopicFilter {
+	/// Matches all topics.
+	Any,
+	/// Matches only statements including all of the given topics.
+	/// Bytes are expected to be a 32-byte topic.
+	MatchAll(Vec<Bytes>),
+}
+
+/// Filter for querying statements with different decryption key identifiers.
+#[derive(Debug, Clone, Serialize, Deserialize)]
+#[serde(rename_all = "camelCase")]
+pub enum DecryptionKeyIdFilter {
+	/// Matches any statement regardless of their decryption key identifier.
+	Any,
+	/// Match only statements without a decryption key identifier.
+	NoDecryptionKey,
+	/// Match only statements with the provided decryption key identifier.
+	/// Bytes are expected to be a 32-byte key identifier.
+	Matches(Bytes),
+}
+
+/// Filter for querying statements with different submitters.
+#[derive(Debug, Clone, Serialize, Deserialize)]
+#[serde(rename_all = "camelCase")]
+pub enum SubmitterFilter {
+	/// Matches any statement regardless of their owner identifier.
+	Any,
+	/// Match only statements with the provided owner identifier.
+	/// Bytes are expected to be a 32-byte owner identifier.
+	Matches(Bytes),
+}
+
+/// Cursor for paginated statement queries.
+#[derive(Debug, Clone, Serialize, Deserialize)]
+pub enum PageCursor {
+	// No more pages
+	End,
+	// Cursor to fetch the next page, opaque to the client.
+	NextPage(Bytes),
+}
+
+#[derive(Debug, Clone, Serialize, Deserialize)]
+#[serde(rename_all = "camelCase")]
+pub struct GetStatementsResponse {
+	/// SCALE-encoded statements matching the query.
+	pub encoded_statements: Vec<Bytes>,
+	/// Cursor for the next page, or `End` if there are no further pages.
+	pub next_page: PageCursor,
+}
+
+/// Substrate statement RPC API
+#[rpc(client, server)]
+pub trait StatementApi2 {
+	/// Return statements that match the provided filters, with pagination.
+	///
+	/// # Parameters
+	///
+	/// - `topic_filter` — Which topics to match. Use `TopicFilter::Any` to match all topics, or
+	///   `TopicFilter::MatchAll(vec)` to match statements that include all provided topics.
+	/// - `key_filter` — Filter by decryption key identifier. Use `DecryptionKeyIdFilter::Any` to
+	///   ignore the decryption key, `NoDecryptionKey` to select statements without a decryption
+	///   key, or `Matches(id)` to select statements with the given key id.
+	/// - `submitter_filter` — Filter by statement submitter. Use `SubmitterFilter::Any` to match
+	///   any owner identifier or `SubmitterFilter::Matches(owner)` to restrict to a specific owner
+	///   identifier.
+	/// - `next_page` — Optional pagination cursor. Pass `None` to request the first page. When a
+	///   previous response contained a `PageCursor::NextPage(bytes)`, pass that wrapped in
+	///   `Some(...)` to fetch the next page. The server will return `PageCursor::End` when there
+	///   are no further pages.
+	/// - `limit` — Optional maximum number of statements to return in this page. The server may
+	///   enforce a maximum cap; if more results exist the response will include a `next_page`
+	///   cursor.
+	///
+	/// # Returns
+	///
+	/// Returns `RpcResult<GetStatementsResponse>` on success.
+	/// - `GetStatementsResponse.encoded_statements` contains a Vec of SCALE-encoded statements as
+	/// `Bytes`.
+	/// - `GetStatementsResponse.next_page` indicates whether more pages are available (an
+	/// opaque cursor) or `End` if the result set is exhausted.
+	#[method(name = "statement_getStatements")]
+	fn get_statements(
+		&self,
+		topic_filter: TopicFilter,
+		key_filter: DecryptionKeyIdFilter,
+		submitter_filter: SubmitterFilter,
+		next_page: Option<PageCursor>,
+		limit: Option<u32>,
+	) -> RpcResult<GetStatementsResponse>;
+
+	/// Subscribe to new statements that match the provided filters.
+	///
+	/// # Parameters
+	///
+	/// See `get_statements` for parameter descriptions.
+	///
+	/// # Returns
+	///
+	/// Returns a stream of SCALE-encoded statements as `Bytes`.
+	#[subscription(
+		name = "statement_subscribeStatement" => "statement_statement",
+		unsubscribe = "statement_unsubscribeStatement",
+		item = Bytes,
+		with_extensions,
+	)]
+	fn subscribe_statement(
+		&self,
+		topic_filter: TopicFilter,
+		key_filter: DecryptionKeyIdFilter,
+		submitter_filter: SubmitterFilter,
+	);
+
+	/// Submit a SCALE-encoded statement.
+	///
+	/// See `Statement` definition for more details.
+	///
+	/// Returns `SubmitResult` indicating success or failure reason.
+	#[method(name = "statement_submit")]
+	fn submit(&self, encoded: Bytes) -> RpcResult<SubmitResult>;
+}
+
 /// Substrate statement RPC API
 #[rpc(client, server)]
+#[deprecated(since = "0.0.0", note = "Please use StatementApi2 instead")]
 pub trait StatementApi {
 	/// Return all statements, SCALE-encoded.
 	#[method(name = "statement_dump", with_extensions)]
@@ -87,7 +216,7 @@ pub trait StatementApi {
 
 	/// Submit a pre-encoded statement.
 	#[method(name = "statement_submit")]
-	fn submit(&self, encoded: Bytes) -> RpcResult<()>;
+	fn submit(&self, encoded: Bytes) -> RpcResult<SubmitResult>;
 
 	/// Remove a statement from the store.
 	#[method(name = "statement_remove")]

substrate/client/rpc/src/statement/mod.rs

@@ -123,18 +123,14 @@ impl StatementApiServer for StatementStore {
 			.collect())
 	}
 
-	fn submit(&self, encoded: Bytes) -> RpcResult<()> {
+	fn submit(&self, encoded: Bytes) -> RpcResult<SubmitResult> {
 		let statement = Decode::decode(&mut &*encoded)
 			.map_err(|e| Error::StatementStore(format!("Error decoding statement: {:?}", e)))?;
 		match self.store.submit(statement, StatementSource::Local) {
-			SubmitResult::New(_) | SubmitResult::Known => Ok(()),
-			// `KnownExpired` should not happen. Expired statements submitted with
-			// `StatementSource::Rpc` should be renewed.
-			SubmitResult::KnownExpired =>
-				Err(Error::StatementStore("Submitted an expired statement.".into()).into()),
-			SubmitResult::Bad(e) => Err(Error::StatementStore(e.into()).into()),
-			SubmitResult::Ignored => Err(Error::StatementStore("Store is full.".into()).into()),
 			SubmitResult::InternalError(e) => Err(Error::StatementStore(e.to_string()).into()),
+			// We return the result as is but `KnownExpired` should not happen. Expired statements
+			// submitted with `StatementSource::Rpc` should be renewed.
+			result => Ok(result),
 		}
 	}
 

substrate/client/statement-store/benches/statement_store.rs

@@ -182,7 +182,7 @@ fn bench_submit(c: &mut Criterion) {
 						s.spawn(move || {
 							for statement in thread_statements {
 								let result = store.submit(statement, StatementSource::Local);
-								assert!(matches!(result, SubmitResult::New(_)));
+								assert!(matches!(result, SubmitResult::New));
 							}
 						});
 					}
@@ -383,7 +383,7 @@ fn bench_mixed_workload(c: &mut Criterion) {
 							for statement in thread_statements {
 								// Submit a statement
 								let result = store.submit(statement, StatementSource::Local);
-								assert!(matches!(result, SubmitResult::New(_)));
+								assert!(matches!(result, SubmitResult::New));
 
 								// Query broadcasts
 								let _ = store.broadcasts(&topics);