statement-store: New RPC result types

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/src/statement/mod.rs

@@ -19,10 +19,73 @@
 //! Substrate Statement Store RPC API.
 
 use jsonrpsee::{core::RpcResult, proc_macros::rpc};
+use serde::{Deserialize, Serialize};
 use sp_core::Bytes;
 
 pub mod error;
 
+/// Result of submitting a statement to the store.
+#[derive(Debug, Clone, Serialize, Deserialize)]
+#[serde(tag = "status", rename_all = "camelCase")]
+pub enum StatementSubmitResult {
+	/// Statement was accepted as new.
+	New,
+	/// Statement was already known.
+	Known,
+	/// Statement was already known but has expired.
+	KnownExpired,
+	/// Statement was rejected because the store is full or priority is too low.
+	Rejected(RejectionReason),
+	/// Statement failed validation.
+	Invalid(InvalidReason),
+}
+
+/// Reason why a statement was rejected from the store.
+#[derive(Debug, Clone, Serialize, Deserialize)]
+#[serde(tag = "reason", rename_all = "camelCase")]
+pub enum RejectionReason {
+	/// Statement data exceeds the maximum allowed size for the account.
+	DataTooLarge {
+		/// The size of the submitted statement data.
+		submitted_size: usize,
+		/// Still available data size for the account.
+		available_size: usize,
+	},
+	/// Attempting to replace a channel message with lower or equal priority.
+	ChannelPriorityTooLow {
+		/// The priority of the submitted statement.
+		submitted_priority: u32,
+		/// The minimum priority of the existing channel message.
+		min_priority: u32,
+	},
+	/// Account reached its statement limit and submitted priority is too low to evict existing.
+	AccountFull {
+		/// The priority of the submitted statement.
+		submitted_priority: u32,
+		/// The minimum priority of the existing statement.
+		min_priority: u32,
+	},
+	/// The global statement store is full and cannot accept new statements.
+	StoreFull,
+}
+
+/// Reason why a statement failed validation.
+#[derive(Debug, Clone, Serialize, Deserialize)]
+#[serde(tag = "reason", rename_all = "camelCase")]
+pub enum InvalidReason {
+	/// Statement has no proof.
+	NoProof,
+	/// Proof validation failed.
+	BadProof,
+	/// Statement exceeds max allowed statement size
+	EncodingTooLarge {
+		/// The size of the submitted statement encoding.
+		submitted_size: usize,
+		/// The maximum allowed size.
+		max_size: usize,
+	},
+}
+
 /// Substrate statement RPC API
 #[rpc(client, server)]
 pub trait StatementApi {
@@ -87,7 +150,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<StatementSubmitResult>;
 
 	/// Remove a statement from the store.
 	#[method(name = "statement_remove")]

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

@@ -24,11 +24,38 @@ use jsonrpsee::{
 	Extensions,
 };
 /// Re-export the API for backward compatibility.
-pub use sc_rpc_api::statement::{error::Error, StatementApiServer};
+pub use sc_rpc_api::statement::{
+	error::Error, InvalidReason, RejectionReason, StatementApiServer, StatementSubmitResult,
+};
 use sp_core::Bytes;
 use sp_statement_store::{StatementSource, SubmitResult};
 use std::sync::Arc;
 
+/// Maps the internal InvalidReason to the RPC API InvalidReason type.
+fn map_invalid_reason(reason: sp_statement_store::InvalidReason) -> InvalidReason {
+	match reason {
+		sp_statement_store::InvalidReason::NoProof => InvalidReason::NoProof,
+		sp_statement_store::InvalidReason::BadProof => InvalidReason::BadProof,
+		sp_statement_store::InvalidReason::EncodingTooLarge { submitted_size, max_size } =>
+			InvalidReason::EncodingTooLarge { submitted_size, max_size },
+	}
+}
+
+/// Maps the internal RejectionReason to the RPC API RejectionReason type.
+fn map_rejection_reason(reason: sp_statement_store::RejectionReason) -> RejectionReason {
+	match reason {
+		sp_statement_store::RejectionReason::DataTooLarge { submitted_size, available_size } =>
+			RejectionReason::DataTooLarge { submitted_size, available_size },
+		sp_statement_store::RejectionReason::ChannelPriorityTooLow {
+			submitted_priority,
+			min_priority,
+		} => RejectionReason::ChannelPriorityTooLow { submitted_priority, min_priority },
+		sp_statement_store::RejectionReason::AccountFull { submitted_priority, min_priority } =>
+			RejectionReason::AccountFull { submitted_priority, min_priority },
+		sp_statement_store::RejectionReason::StoreFull => RejectionReason::StoreFull,
+	}
+}
+
 /// Statement store API
 pub struct StatementStore {
 	store: Arc<dyn sp_statement_store::StatementStore>,
@@ -123,17 +150,19 @@ impl StatementApiServer for StatementStore {
 			.collect())
 	}
 
-	fn submit(&self, encoded: Bytes) -> RpcResult<()> {
+	fn submit(&self, encoded: Bytes) -> RpcResult<StatementSubmitResult> {
 		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(()),
+			SubmitResult::New => Ok(StatementSubmitResult::New),
+			SubmitResult::Known => Ok(StatementSubmitResult::Known),
 			// `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::KnownExpired => Ok(StatementSubmitResult::KnownExpired),
+			SubmitResult::Rejected(reason) =>
+				Ok(StatementSubmitResult::Rejected(map_rejection_reason(reason))),
+			SubmitResult::Invalid(reason) =>
+				Ok(StatementSubmitResult::Invalid(map_invalid_reason(reason))),
 			SubmitResult::InternalError(e) => Err(Error::StatementStore(e.to_string()).into()),
 		}
 	}

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);