functionland · evilrobot-01 · Aug 3, 2023
diff --git a/crates/sugarfunge-api-types/Cargo.toml b/crates/sugarfunge-api-types/Cargo.toml
@@ -3,15 +3,19 @@ name = "sugarfunge-api-types"
 version = "0.1.0"
 edition = "2021"
 
+#SBP-M1 review: remove comment
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
 
 [dependencies]
 serde = { version = "1.0", features = ["derive"] }
 serde_json = "1.0"
 codec = { package = "parity-scale-codec", version = "3.0.0", default-features = false, features = ["derive", "full", "bit-vec"] }
 scale-info = { version = "2.0.0", features = ["bit-vec"] }
+#SBP-M1 review: consider updating
 bevy_derive = "0.10"
 
+#SBP-M1 review: use inline dependencies as above, rather than [dependencoes.X]
+#SBP-M1 review: assumed subxt to be somewhat version agnostic, relying on metadata retrieved from the node for extrinsic encoding/decoding. Consider whether this fork is really required.
 [dependencies.subxt]
 git = "https://github.com/SugarFunge/subxt.git"
 branch = "feature/polkadot-v0.9.42"

diff --git a/crates/sugarfunge-api-types/src/account.rs b/crates/sugarfunge-api-types/src/account.rs
@@ -1,17 +1,20 @@
 use crate::primitives::*;
 use serde::{Deserialize, Serialize};
 
+// SBP-M1 review: hardcoded values seem problematic when progressing beyond dev-chain as dev accounts are well known
 pub const REFUND_SEED: &str = "//Alice";
 pub const REFUND_FEE_VALUE: u128 = 20000000000000000;
 
 #[derive(Serialize, Deserialize, Debug)]
 pub struct CreateAccountOutput {
+    // SBP-M1 review: remove seed
     pub seed: Seed,
     pub account: Account,
 }
 
 #[derive(Serialize, Deserialize, Debug)]
 pub struct FundAccountInput {
+    // SBP-M1 review: remove seed
     pub seed: Seed,
     pub to: Account,
     pub amount: Balance,
@@ -47,6 +50,7 @@ pub struct AccountExistsOutput {
 
 #[derive(Serialize, Deserialize, Debug)]
 pub struct SeededAccountInput {
+    // SBP-M1 review: remove seed
     pub seed: Seed,
 }
 

diff --git a/crates/sugarfunge-api-types/src/asset.rs b/crates/sugarfunge-api-types/src/asset.rs
@@ -3,6 +3,7 @@ use serde::{Deserialize, Serialize};
 
 #[derive(Serialize, Deserialize, Debug)]
 pub struct CreateClassInput {
+    // SBP-M1 review: remove seed
     pub seed: Seed,
     pub class_id: ClassId,
     pub metadata: serde_json::Value,
@@ -34,6 +35,7 @@ pub struct ClassInfoOutput {
 
 #[derive(Serialize, Deserialize, Debug)]
 pub struct CreateInput {
+    // SBP-M1 review: remove seed
     pub seed: Seed,
     pub class_id: ClassId,
     pub asset_id: AssetId,
@@ -67,6 +69,7 @@ pub struct AssetInfoOutput {
 
 #[derive(Serialize, Deserialize, Debug)]
 pub struct UpdateMetadataInput {
+    // SBP-M1 review: remove seed
     pub seed: Seed,
     pub class_id: ClassId,
     pub asset_id: AssetId,
@@ -83,6 +86,7 @@ pub struct UpdateMetadataOutput {
 
 #[derive(Serialize, Deserialize, Debug)]
 pub struct MintInput {
+    // SBP-M1 review: remove seed
     pub seed: Seed,
     pub to: Account,
     pub class_id: ClassId,
@@ -101,6 +105,7 @@ pub struct MintOutput {
 
 #[derive(Serialize, Deserialize, Debug)]
 pub struct BurnInput {
+    // SBP-M1 review: remove seed
     pub seed: Seed,
     pub from: Account,
     pub class_id: ClassId,
@@ -150,6 +155,7 @@ pub struct AssetBalanceItemOutput {
 
 #[derive(Serialize, Deserialize, Debug)]
 pub struct TransferFromInput {
+    // SBP-M1 review: remove seed
     pub seed: Seed,
     pub from: Account,
     pub to: Account,

diff --git a/crates/sugarfunge-api-types/src/bag.rs b/crates/sugarfunge-api-types/src/bag.rs
@@ -3,6 +3,7 @@ use serde::{Deserialize, Serialize};
 
 #[derive(Serialize, Deserialize, Debug)]
 pub struct RegisterInput {
+    // SBP-M1 review: remove seed
     pub seed: Seed,
     pub class_id: ClassId,
     pub metadata: serde_json::Value,
@@ -16,6 +17,7 @@ pub struct RegisterOutput {
 
 #[derive(Serialize, Deserialize, Debug)]
 pub struct CreateInput {
+    // SBP-M1 review: remove seed
     pub seed: Seed,
     pub class_id: ClassId,
     pub owners: Vec<Account>,
@@ -32,6 +34,7 @@ pub struct CreateOutput {
 
 #[derive(Serialize, Deserialize, Debug)]
 pub struct SweepInput {
+    // SBP-M1 review: remove seed
     pub seed: Seed,
     pub bag: Account,
     pub to: Account,
@@ -46,6 +49,7 @@ pub struct SweepOutput {
 
 #[derive(Serialize, Deserialize, Debug)]
 pub struct DepositInput {
+    // SBP-M1 review: remove seed
     pub seed: Seed,
     pub bag: Account,
     pub class_ids: Vec<ClassId>,

diff --git a/crates/sugarfunge-api-types/src/bundle.rs b/crates/sugarfunge-api-types/src/bundle.rs
@@ -10,6 +10,7 @@ pub struct BundleSchema {
 
 #[derive(Serialize, Deserialize, Debug)]
 pub struct RegisterBundleInput {
+    // SBP-M1 review: remove seed
     pub seed: Seed,
     pub class_id: ClassId,
     pub asset_id: AssetId,
@@ -27,6 +28,7 @@ pub struct RegisterBundleOutput {
 
 #[derive(Serialize, Deserialize, Debug)]
 pub struct MintBundleInput {
+    // SBP-M1 review: remove seed
     pub seed: Seed,
     pub from: Account,
     pub to: Account,
@@ -45,6 +47,7 @@ pub struct MintBundleOutput {
 
 #[derive(Serialize, Deserialize, Debug)]
 pub struct BurnBundleInput {
+    // SBP-M1 review: remove seed
     pub seed: Seed,
     pub from: Account,
     pub to: Account,

diff --git a/crates/sugarfunge-api-types/src/challenge.rs b/crates/sugarfunge-api-types/src/challenge.rs
@@ -23,6 +23,7 @@ impl Into<ChallengeStateValue> for ChallengeState {
 
 #[derive(Serialize, Deserialize, Debug)]
 pub struct GenerateChallengeInput {
+    // SBP-M1 review: remove seed
     pub seed: Seed,
 }
 
@@ -38,6 +39,7 @@ pub struct GenerateChallengeOutput {
 
 #[derive(Serialize, Deserialize, Debug)]
 pub struct VerifyChallengeInput {
+    // SBP-M1 review: remove seed
     pub seed: Seed,
     pub pool_id: PoolId,
     pub cids: Vec<Cid>,
@@ -56,6 +58,7 @@ pub struct VerifyChallengeOutput {
 
 #[derive(Serialize, Deserialize, Debug)]
 pub struct MintLaborTokensInput {
+    // SBP-M1 review: remove seed
     pub seed: Seed,
     pub class_id: ClassId,
     pub asset_id: AssetId,
@@ -100,6 +103,7 @@ pub struct VerifyFileSizeOutput {
 
 #[derive(Serialize, Deserialize, Debug)]
 pub struct ProvideFileSizeInput {
+    // SBP-M1 review: remove seed
     pub seed: Seed,
     pub pool_id: PoolId,
     pub cids: Vec<Cid>,

diff --git a/crates/sugarfunge-api-types/src/contract.rs b/crates/sugarfunge-api-types/src/contract.rs
@@ -26,6 +26,7 @@ pub struct ContractAllowanceOutput {
 
 #[derive(Serialize, Deserialize, Debug)]
 pub struct ConvertFulaInput {
+    // SBP-M1 review: remove seed
     pub seed: Seed,
     pub wallet_account: String,
     pub amount: Balance,

diff --git a/crates/sugarfunge-api-types/src/fula.rs b/crates/sugarfunge-api-types/src/fula.rs
@@ -5,6 +5,7 @@ use serde::{Deserialize, Serialize};
 
 #[derive(Serialize, Deserialize, Debug)]
 pub struct VerifyManifestsInput {
+    // SBP-M1 review: remove seed
     pub seed: Seed,
 }
 
@@ -19,6 +20,7 @@ pub struct VerifyManifestsOutput {
 
 #[derive(Serialize, Deserialize, Debug)]
 pub struct UploadManifestInput {
+    // SBP-M1 review: remove seed
     pub seed: Seed,
     pub manifest_metadata: serde_json::Value,
     pub cid: Cid,
@@ -38,6 +40,7 @@ pub struct UploadManifestOutput {
 
 #[derive(Serialize, Deserialize, Debug)]
 pub struct BatchUploadManifestInput {
+    // SBP-M1 review: remove seed
     pub seed: Seed,
     pub manifest_metadata: Vec<serde_json::Value>,
     pub cid: Vec<Cid>,
@@ -56,6 +59,7 @@ pub struct BatchUploadManifestOutput {
 
 #[derive(Serialize, Deserialize, Debug)]
 pub struct UpdateManifestInput {
+    // SBP-M1 review: remove seed
     pub seed: Seed,
     pub cid: Cid,
     pub pool_id: PoolId,
@@ -78,6 +82,7 @@ pub struct UpdatedManifestOutput {
 
 #[derive(Serialize, Deserialize, Debug)]
 pub struct StorageManifestInput {
+    // SBP-M1 review: remove seed
     pub seed: Seed,
     pub cid: Cid,
     pub pool_id: PoolId,
@@ -94,6 +99,7 @@ pub struct StorageManifestOutput {
 
 #[derive(Serialize, Deserialize, Debug)]
 pub struct BatchStorageManifestInput {
+    // SBP-M1 review: remove seed
     pub seed: Seed,
     pub pool_id: PoolId,
     pub cid: Vec<Cid>,
@@ -110,6 +116,7 @@ pub struct BatchStorageManifestOutput {
 
 #[derive(Serialize, Deserialize, Debug)]
 pub struct RemoveManifestInput {
+    // SBP-M1 review: remove seed
     pub seed: Seed,
     pub cid: Cid,
     pub pool_id: PoolId,
@@ -125,6 +132,7 @@ pub struct RemoveManifestOutput {
 
 #[derive(Serialize, Deserialize, Debug)]
 pub struct BatchRemoveManifestInput {
+    // SBP-M1 review: remove seed
     pub seed: Seed,
     pub pool_id: Vec<PoolId>,
     pub cid: Vec<Cid>,
@@ -140,6 +148,7 @@ pub struct BatchRemoveManifestOutput {
 
 #[derive(Serialize, Deserialize, Debug)]
 pub struct RemoveStoringManifestInput {
+    // SBP-M1 review: remove seed
     pub seed: Seed,
     pub cid: Cid,
     pub pool_id: PoolId,
@@ -155,6 +164,7 @@ pub struct RemoveStoringManifestOutput {
 
 #[derive(Serialize, Deserialize, Debug)]
 pub struct BatchRemoveStoringManifestInput {
+    // SBP-M1 review: remove seed
     pub seed: Seed,
     pub pool_id: PoolId,
     pub cid: Vec<Cid>,

diff --git a/crates/sugarfunge-api-types/src/lib.rs b/crates/sugarfunge-api-types/src/lib.rs
@@ -1,3 +1,4 @@
+// SBP-M1 review: add doc comments to all types and fields, which can be used for generation API docs
 #[subxt::subxt(
     runtime_metadata_path = "sugarfunge_metadata.scale",
     derive_for_type(
@@ -20,3 +21,6 @@ pub mod market;
 pub mod pool;
 pub mod primitives;
 pub mod validator;
+
+// SBP-M1 review: this crate uses subxt as a dependency and is itself used by the proof-engine.
+// SBP-M1 review: A recommendation is to add a simple API to this crate which facilitates signing of extrinsics on the 'client', rather than within the 'sugarfunge-api' via seeds being passed around.
diff --git a/crates/sugarfunge-api-types/src/market.rs b/crates/sugarfunge-api-types/src/market.rs
@@ -1,3 +1,4 @@
+// SBP-M1 review: no tests
 use crate::primitives::*;
 use serde::{Deserialize, Serialize};
 
@@ -210,6 +211,7 @@ pub struct Rates {
 
 #[derive(Serialize, Deserialize, Debug)]
 pub struct CreateMarketInput {
+    // SBP-M1 review: remove seed
     pub seed: Seed,
     pub market_id: MarketId,
 }
@@ -222,6 +224,7 @@ pub struct CreateMarketOutput {
 
 #[derive(Serialize, Deserialize, Debug)]
 pub struct CreateMarketRateInput {
+    // SBP-M1 review: remove seed
     pub seed: Seed,
     pub market_id: MarketId,
     pub market_rate_id: MarketId,
@@ -236,6 +239,7 @@ pub struct CreateMarketRateOutput {
 
 #[derive(Serialize, Deserialize, Debug)]
 pub struct DepositAssetsInput {
+    // SBP-M1 review: remove seed
     pub seed: Seed,
     pub market_id: MarketId,
     pub market_rate_id: MarketId,
@@ -254,6 +258,7 @@ pub struct DepositAssetsOutput {
 
 #[derive(Serialize, Deserialize, Debug)]
 pub struct ExchangeAssetsInput {
+    // SBP-M1 review: remove seed
     pub seed: Seed,
     pub market_id: MarketId,
     pub market_rate_id: MarketId,

diff --git a/crates/sugarfunge-api-types/src/pool.rs b/crates/sugarfunge-api-types/src/pool.rs
@@ -5,6 +5,7 @@ use serde::{Deserialize, Serialize};
 
 #[derive(Serialize, Deserialize, Debug)]
 pub struct CreatePoolInput {
+    // SBP-M1 review: remove seed
     pub seed: Seed,
     pub pool_name: Name,
     pub peer_id: PeerId,
@@ -21,6 +22,7 @@ pub struct CreatePoolOutput {
 
 #[derive(Serialize, Deserialize, Debug)]
 pub struct LeavePoolInput {
+    // SBP-M1 review: remove seed
     pub seed: Seed,
     pub pool_id: PoolId,
 }
@@ -35,6 +37,7 @@ pub struct LeavePoolOutput {
 
 #[derive(Serialize, Deserialize, Debug)]
 pub struct JoinPoolInput {
+    // SBP-M1 review: remove seed
     pub seed: Seed,
     pub pool_id: PoolId,
     pub peer_id: PeerId,
@@ -50,6 +53,7 @@ pub struct JoinPoolOutput {
 
 #[derive(Serialize, Deserialize, Debug)]
 pub struct CancelJoinPoolInput {
+    // SBP-M1 review: remove seed
     pub seed: Seed,
     pub pool_id: PoolId,
 }
@@ -64,6 +68,7 @@ pub struct CancelJoinPoolOutput {
 
 #[derive(Serialize, Deserialize, Debug)]
 pub struct VoteInput {
+    // SBP-M1 review: remove seed
     pub seed: Seed,
     pub pool_id: PoolId,
     pub account: Account,

diff --git a/crates/sugarfunge-api-types/src/primitives.rs b/crates/sugarfunge-api-types/src/primitives.rs
@@ -1,3 +1,5 @@
+// SBP-M1 review: group use statements without line breaks
+// SBP-M1 review: no tests
 use std::{ops::Div, str::FromStr};
 
 use serde::{Deserialize, Serialize};
@@ -9,6 +11,8 @@ use sp_core::U256;
 use bevy_derive::{Deref, DerefMut};
 
 #[derive(Serialize, Deserialize, Clone, Debug, Deref, DerefMut)]
+// SBP-M1 review: remove type, seed should not be passed to an API for signing but rather receive a signed extrinsic/transaction for passing to the chain for execution
+// SBP-M1 review: look at how https://github.com/paritytech/substrate-api-sidecar implements this at https://paritytech.github.io/substrate-api-sidecar/dist/
 pub struct Seed(String);
 
 impl From<String> for Seed {
@@ -297,6 +301,7 @@ impl From<ReplicationFactor> for u16 {
     }
 }
 
+// SBP-M1 review: consider replacing the below helper methods with From/Into trait implementations where possible, so transformation can be simply achieved via .into()
 pub fn transform_vec_account_to_string(in_vec: Vec<Account>) -> Vec<String> {
     in_vec
         .into_iter()