Rules #22

CyonAlexRDX · 2024-11-27T07:58:04Z

Important

Implementation of rules laid out in doc
I have NOT yet implemented "Automatic Security Shield Construction"
That will come in a coming PR.

Note

LOC delta is misleading, of the actual 5k code, 3k is unit tests, so "only" cirka 2k lines of PROD code.

codecov · 2024-11-27T08:17:47Z

Codecov Report

Attention: Patch coverage is 93.81663% with 58 lines in your changes missing coverage. Please review.

Project coverage is 93.8%. Comparing base (a3d1fa6) to head (146fd13).
Report is 35 commits behind head on main.

Files with missing lines	Patch %	Lines
crates/rules/src/roles/builder/roles_builder.rs	85.3%	25 Missing ⚠️
...ith_hierarchical_deterministic_factor_instances.rs	79.6%	11 Missing ⚠️
crates/rules-uniffi/src/builder.rs	90.3%	10 Missing ⚠️
...actor_instance_level/role_with_factor_instances.rs	77.1%	8 Missing ⚠️
.../rules/src/roles/abstract_role_builder_or_built.rs	93.0%	3 Missing ⚠️
...s/factor_source_level/roles_with_factor_sources.rs	92.8%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##           main     #22      +/-   ##
=======================================
+ Coverage      0   93.8%   +93.8%     
=======================================
  Files         0      24      +24     
  Lines         0     938     +938     
=======================================
+ Hits          0     880     +880     
- Misses        0      58      +58

Files with missing lines	Coverage Δ
...factor_source_in_role_builder_validation_status.rs	`100.0% <100.0%> (ø)`
...s/src/matrices/abstract_matrix_builder_or_built.rs	`100.0% <100.0%> (ø)`
crates/rules/src/matrices/builder/error.rs	`100.0% <100.0%> (ø)`
...rates/rules/src/matrices/builder/matrix_builder.rs	`100.0% <100.0%> (ø)`
...s/rules/src/matrices/matrix_of_factor_instances.rs	`100.0% <100.0%> (ø)`
.../rules/src/matrices/matrix_of_factor_source_ids.rs	`100.0% <100.0%> (ø)`
...tes/rules/src/matrices/matrix_of_factor_sources.rs	`100.0% <100.0%> (ø)`
...e_level/confirmation_role_with_factor_instances.rs	`100.0% <100.0%> (ø)`
...stance_level/primary_role_with_factor_instances.rs	`100.0% <100.0%> (ø)`
...tance_level/recovery_role_with_factor_instances.rs	`100.0% <100.0%> (ø)`
... and 14 more

… of roles and matrices - with UniFFI implementation.

Sajjon · 2024-11-27T09:21:17Z

crates/rules-uniffi/src/builder.rs

+#[uniffi::export(Debug, Eq, Hash)]
+pub struct SecurityStructureOfFactorSourceIds {
+    wrapped_matrix: MatrixWithFactorSourceIds,
+    name: String,


this will have an ID as well... the models here in this crate is not 100% accurate yet. What I want reviewed is probably just the implementation of the wrapping? And the API

Sajjon · 2024-11-27T09:21:31Z

crates/rules-uniffi/src/builder.rs

+        self.with(|builder| builder.add_factor_source_to_primary_threshold(factor_source_id.inner))
+    }
+
+    pub fn add_factor_source_to_primary_override(


TODO doc all methods ofc...

Sajjon · 2024-11-27T09:22:02Z

crates/rules-uniffi/src/builder.rs

Im about to write unit tests for this file/cratel Currently I just have one ensuring it works...

Sajjon · 2024-11-27T09:22:32Z

crates/rules/src/matrices/abstract_matrix_builder_or_built.rs

TODO: Document

Sajjon · 2024-11-27T09:23:42Z

crates/rules/src/matrices/builder/matrix_builder.rs

TODO Document API

Sajjon · 2024-11-27T09:24:01Z

crates/rules/src/matrices/builder/matrix_builder_unit_tests.rs

Dont miss, many tests here :)

Sajjon · 2024-11-27T09:24:19Z

crates/rules/src/matrices/matrix_with_factor_source_ids.rs

TODO: Add unit tests for JSON !

Sajjon · 2024-11-27T09:24:31Z

crates/rules/src/move_to_sargon.rs

irrelevant file, can skip review

Sajjon · 2024-11-27T09:24:43Z

crates/rules/src/roles/abstract_role_builder_or_built.rs

+use crate::prelude::*;
+
+#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)]
+pub struct AbstractRoleBuilderOrBuilt<F, T> {


TODO Document!

Sajjon · 2024-11-27T09:25:26Z

crates/rules/src/roles/abstract_role_builder_or_built.rs

+    }
+}
+
+impl RoleBuilder {


neither method on the RoleBuilder is meant to be pub the idea is that hosts always use a single MatrixBuilder (ShieldBuilder)` and which expose the needed APIs.

Sajjon · 2024-11-27T09:26:47Z

crates/rules/src/roles/builder/roles_builder.rs

+}
+
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, thiserror::Error)]
+pub enum NotYetValidReason {


In earlier implementations I had one error enum type for each Role, maybe that makes sense, not sure. It results in many more types and more complex translation into RoleBuilderValidation so I deemed it not worth it.

Furtermore, all these violations are "ephemeral" in the sense that they ALL will be translated into CommonError at some point in time...

Sajjon · 2024-11-27T09:27:06Z

crates/rules/src/roles/builder/roles_builder.rs

+}
+
+#[derive(Debug, Clone, PartialEq, Eq, Hash)]
+pub struct FactorSourceInRoleBuilderValidationStatus {


TODO: Split out and document

Sajjon · 2024-11-27T09:27:23Z

crates/rules/src/roles/builder/roles_builder.rs

+use NotYetValidReason::*;
+use RoleKind::*;
+
+impl RoleBuilder {


TODO: Document

Sajjon · 2024-11-27T09:27:43Z

crates/rules/src/roles/builder/roles_builder_unit_tests.rs

dont miss, many tests :)

GhenadieVP

Overall as a structure this looks very good 😍!
I didn't review the very thoroughly in regards to how the rules logic is applied, let me know if you want that.

So I have only one comment to clarify.

crates/rules/src/matrices/builder/matrix_builder.rs

… for validate/build.

Const role

CyonAlexRDX force-pushed the main branch from 37b78fc to a3d1fa6 Compare November 27, 2024 07:58

CyonAlexRDX force-pushed the rules branch from 090401a to 2d3e8cc Compare November 27, 2024 08:04

CyonAlexRDX force-pushed the rules branch 2 times, most recently from 7a9e0e2 to 40acef4 Compare November 27, 2024 08:29

Add rules and rules-uniffi crates with implementation of builders…

b4dd55b

… of roles and matrices - with UniFFI implementation.

CyonAlexRDX force-pushed the rules branch from 40acef4 to b4dd55b Compare November 27, 2024 08:53