Improve API-related things by dr-orlovsky · Pull Request #24 · AluVM/sonic

dr-orlovsky · 2025-05-25T11:32:38Z

No description provided.

codecov · 2025-05-25T11:34:02Z

Codecov Report

Attention: Patch coverage is 76.65505% with 201 lines in your changes missing coverage. Please review.

Project coverage is 75.9%. Comparing base (706089f) to head (c74a154).
Report is 42 commits behind head on master.

Files with missing lines	Patch %	Lines
api/src/state/aggregators.rs	79.9%	48 Missing ⚠️
api/src/api.rs	75.2%	32 Missing ⚠️
api/src/state/adaptors.rs	72.3%	31 Missing ⚠️
api/src/articles.rs	59.1%	27 Missing ⚠️
api/src/builders.rs	56.1%	25 Missing ⚠️
src/ledger.rs	86.7%	12 Missing ⚠️
api/src/issuer.rs	86.7%	10 Missing ⚠️
src/state.rs	82.6%	8 Missing ⚠️
api/src/state/arithmetics.rs	0.0%	5 Missing ⚠️
api/src/state/raw.rs	0.0%	2 Missing ⚠️
... and 1 more

Additional details and impacted files

@@           Coverage Diff            @@
##           master     #24     +/-   ##
========================================
+ Coverage    70.4%   75.9%   +5.5%     
========================================
  Files          18      18             
  Lines        1732    2254    +522     
========================================
+ Hits         1219    1711    +492     
- Misses        513     543     +30

Flag	Coverage Δ
rust	`75.9% <76.7%> (+5.5%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

…ements

…mpatible

Signed-off-by: Dr Maxim Orlovsky <orlovsky@lnp-bp.org>

Bring changes from ultrasonic RC2

dr-orlovsky · 2025-06-01T18:12:39Z

@codecov-ai-reviewer review

ghost · 2025-06-01T18:12:44Z

On it! We are reviewing the PR and will provide feedback shortly.

ghost · 2025-06-01T18:16:01Z

PR Description

This pull request refactors the Sonic API to improve its structure, flexibility, and maintainability. It introduces a new Semantics struct to encapsulate contract API definitions, improves versioning and signature validation, and updates state management to align with the new API design.

Click to see more

Key Technical Changes

API Restructuring: Replaces ImmutableApi and DestructibleApi with GlobalApi and OwnedApi, respectively, for clearer semantics. Introduces Semantics struct to group API definitions, codex libraries, and type systems.
Versioning and Checksums: Adds ApisChecksum and ArticlesId for improved API versioning and identification. Introduces ParseVersionedError for handling version parsing errors.
Signature Validation: Implements signature validation in Articles::with and Issuer::with using a provided validator function.
State Management: Updates state management in EffectiveState and ProcessedState to reflect the new API structure, including changes to how state is applied and rolled back.
Aggregator Refactoring: Refactors StateAggregator into Aggregator and SubAggregator for more flexible state aggregation.
Dependency Updates: Updates dependencies in Cargo.toml and Cargo.lock, including adding once_cell_polyfill and indexmap.

Architecture Decisions

Semantics Struct: The decision to introduce the Semantics struct centralizes API-related data, improving code organization and reducing redundancy between Issuer and Articles. This promotes a more modular design.
Signature Validation Abstraction: The use of a signature validator function allows for greater flexibility in signature schemes and validation logic.
Global/Owned State: The renaming of state types to Global and Owned clarifies the intended usage and lifecycle of contract state.

Dependencies and Interactions

AluVM: The AluVM variants in StateConvertor, StateBuilder, and StateArithm are currently unimplemented and marked as 'reserved for the future'. This indicates a planned integration with the AluVM, but it's important to ensure that these placeholders don't introduce unexpected behavior or dependencies.
StrictEncoding/StrictTypes: The changes rely heavily on strict_encoding and strict_types for serialization and type definitions. Any changes in these dependencies could impact the API's stability.
Ultrasonic: The refactoring is tightly coupled with the ultrasonic crate, particularly its data structures for representing contract state and operations. Reviewers should pay close attention to how these interactions are affected.

Risk Considerations

Breaking Changes: The refactoring introduces several breaking changes, particularly in the API structure and naming conventions. A comprehensive migration guide is essential to minimize disruption for existing users.
Signature Validation: The signature validation logic relies on external validator functions. It's crucial to ensure that these functions are thoroughly tested and secure to prevent vulnerabilities.
AluVM Integration: The unimplemented AluVM variants could introduce unexpected behavior or dependencies if not handled carefully. Consider using feature flags to disable these variants until they are fully implemented.
Performance: The changes to state management and aggregation could potentially impact performance. Thorough benchmarking is recommended to identify and address any performance regressions.

Notable Implementation Details

ApisChecksum: The ApisChecksum is explicitly marked as non-unique and not for verification. This is a critical detail that should be carefully validated to prevent misuse.
SemanticError: The SemanticError enum provides a structured way to handle errors related to contract semantics. Reviewers should ensure that all relevant error conditions are covered by this enum.
State Conversion and Building: The typed_convert and typed_build functions in api/src/state/adaptors.rs are complex and require careful attention to detail to ensure correct state handling.

ghost · 2025-06-01T18:16:05Z


-//! API defines how a contract can be interfaced by software.
+//! API defines how software can interface a contract.


Great improvement to the API documentation in the comment change from 'API defines how a contract can be interfaced by software' to 'API defines how software can interface a contract'. The new phrasing emphasizes the software's role in interfacing with contracts, which better aligns with the purpose of the API.

ghost · 2025-06-01T18:16:08Z

+/// It is created just for UI, so users can easily visually distinguish different sets of APIs from
+/// each other.
+///
+/// This type is not - and must not be used in any verification.
+#[derive(Wrapper, Copy, Clone, Ord, PartialOrd, Eq, PartialEq, Hash, Debug, From)]
+#[wrapper(Deref, BorrowSlice, Hex, Index, RangeOps)]
+#[derive(StrictType, StrictDumb, StrictEncode, StrictDecode)]
+#[strict_type(lib = LIB_NAME_SONIC)]
+pub struct ApisChecksum(
+    #[from]


The ApisChecksum type is marked as 'not a unique identifier' and specifically mentioned as 'not to be used in any verification'. This warning should be more prominent, perhaps with additional validation in code to ensure it's not accidentally used for verification purposes. Consider adding a method that explicitly validates its intended use.

ghost · 2025-06-01T18:16:10Z

+            let lib = lib_map.get(id).ok_or(SemanticError::MissedCodexLib(*id))?;
+            lib_ids.extend(lib.libs.iter().copied());
+            count = lib_ids.len();
+            i += 1;
+        }
+        for id in lib_map.keys() {
+            if !lib_ids.contains(id) {
+                return Err(SemanticError::ExcessiveCodexLib(*id));
+            }
+        }
+
+        // Check API libs for redundancies and completeness
+        let lib_map = self
+            .api_libs
+            .iter()
+            .map(|lib| (lib.lib_id(), lib))
+            .collect::<IndexMap<_, _>>();
+
+        let mut lib_ids = indexset![];
+        for api in self.apis() {
+            for agg in api.aggregators.values() {
+                if let Aggregator::AluVM(entry) = agg {
+                    lib_ids.insert(entry.lib_id);
+                }
+            }
+            for glob in api.global.values() {
+                if let StateConvertor::AluVM(entry) = glob.convertor {
+                    lib_ids.insert(entry.lib_id);
+                }
+                if let StateBuilder::AluVM(entry) = glob.builder {
+                    lib_ids.insert(entry.lib_id);
+                }
+                if let RawConvertor::AluVM(entry) = glob.raw_convertor {
+                    lib_ids.insert(entry.lib_id);
+                }
+                if let RawBuilder::AluVM(entry) = glob.raw_builder {
+                    lib_ids.insert(entry.lib_id);
+                }
+            }
+            for owned in api.owned.values() {
+                if let StateConvertor::AluVM(entry) = owned.convertor {
+                    lib_ids.insert(entry.lib_id);
+                }
+                if let StateBuilder::AluVM(entry) = owned.builder {
+                    lib_ids.insert(entry.lib_id);
+                }
+                if let StateBuilder::AluVM(entry) = owned.witness_builder {
+                    lib_ids.insert(entry.lib_id);
+                }
+                if let StateArithm::AluVM(entry) = owned.arithmetics {
+                    lib_ids.insert(entry.lib_id);
+                }
+            }
+        }
+        let mut i = 0usize;
+        let mut count = lib_ids.len();
+        while i < count {
+            let id = lib_ids.get_index(i).expect("index is valid");
+            let lib = lib_map.get(id).ok_or(SemanticError::MissedApiLib(*id))?;


The refactoring from 'immutable'/'destructible' to 'global'/'owned' improves semantic clarity, making the code more self-explanatory. This is a good change for maintainability. However, ensure this breaking change is well-documented in the migration guide for existing users.

ghost · 2025-06-01T18:16:12Z

+        e.commit_to_hash(&self.default);
+        // We do not commit to the codex_libs since they are not a part of APIs
+        // and are commit to inside the codex.
+        // The fact that there are no other libs
+        // is verified in the Articles and Issuer constructors.
+        let apis = SmallOrdMap::from_iter_checked(
+            self.custom
+                .iter()
+                .map(|(name, api)| (name.clone(), api.api_id())),
+        );
+        e.commit_to_linear_map(&apis);
+        let libs = SmallOrdSet::from_iter_checked(self.api_libs.iter().map(Lib::lib_id));
+        e.commit_to_linear_set(&libs);
+        e.commit_to_serialized(&self.types.id());
+    }
+}
+
+impl Semantics {
+    pub fn apis_checksum(&self) -> ApisChecksum { self.commit_id() }
+
+    /// Iterates over all APIs, including default and named ones.
+    pub fn apis(&self) -> impl Iterator<Item = &Api> { [&self.default].into_iter().chain(self.custom.values()) }
+
+    /// Check whether this semantics object matches codex and the provided set of libraries for it.
+    pub fn check(&self, codex: &Codex) -> Result<(), SemanticError> {
+        let codex_id = codex.codex_id();
+
+        let mut ids = bset![];
+        for api in self.apis() {
+            if api.codex_id != codex_id {
+                return Err(SemanticError::CodexMismatch);
+            }
+            let api_id = api.api_id();
+            if !ids.insert(api_id) {
+                return Err(SemanticError::DuplicatedApi(api_id));
+            }
+        }
+
+        // Check codex libs for redundancies and completeness
+        let lib_map = self
+            .codex_libs
+            .iter()
+            .map(|lib| (lib.lib_id(), lib))
+            .collect::<IndexMap<_, _>>();
+
+        let mut lib_ids = codex
+            .verifiers
+            .values()
+            .map(|entry| entry.lib_id)
+            .collect::<IndexSet<_>>();
+        let mut i = 0usize;
+        let mut count = lib_ids.len();
+        while i < count {
+            let id = lib_ids.get_index(i).expect("index is valid");
+            let lib = lib_map.get(id).ok_or(SemanticError::MissedCodexLib(*id))?;
+            lib_ids.extend(lib.libs.iter().copied());
+            count = lib_ids.len();
+            i += 1;
+        }
+        for id in lib_map.keys() {
+            if !lib_ids.contains(id) {
+                return Err(SemanticError::ExcessiveCodexLib(*id));
+            }
+        }
+
+        // Check API libs for redundancies and completeness
+        let lib_map = self
+            .api_libs
+            .iter()
+            .map(|lib| (lib.lib_id(), lib))
+            .collect::<IndexMap<_, _>>();
+
+        let mut lib_ids = indexset![];
+        for api in self.apis() {
+            for agg in api.aggregators.values() {
+                if let Aggregator::AluVM(entry) = agg {
+                    lib_ids.insert(entry.lib_id);


The SubAggregator enum has many variants that perform similar mathematical operations but with different error handling (e.g., SumUnwrap vs SumOrDefault). Consider adding shared implementation for these related operations to reduce code duplication and improve maintenance.

ghost · 2025-06-01T18:16:13Z

+    fn commit_encode(&self, e: &mut CommitEngine) {
+        e.commit_to_serialized(&self.version);
+        e.commit_to_hash(&self.default);
+        // We do not commit to the codex_libs since they are not a part of APIs
+        // and are commit to inside the codex.
+        // The fact that there are no other libs
+        // is verified in the Articles and Issuer constructors.
+        let apis = SmallOrdMap::from_iter_checked(
+            self.custom
+                .iter()
+                .map(|(name, api)| (name.clone(), api.api_id())),
+        );
+        e.commit_to_linear_map(&apis);
+        let libs = SmallOrdSet::from_iter_checked(self.api_libs.iter().map(Lib::lib_id));
+        e.commit_to_linear_set(&libs);
+        e.commit_to_serialized(&self.types.id());
+    }
+}
+
+impl Semantics {
+    pub fn apis_checksum(&self) -> ApisChecksum { self.commit_id() }
+
+    /// Iterates over all APIs, including default and named ones.
+    pub fn apis(&self) -> impl Iterator<Item = &Api> { [&self.default].into_iter().chain(self.custom.values()) }
+
+    /// Check whether this semantics object matches codex and the provided set of libraries for it.
+    pub fn check(&self, codex: &Codex) -> Result<(), SemanticError> {
+        let codex_id = codex.codex_id();
+
+        let mut ids = bset![];
+        for api in self.apis() {
+            if api.codex_id != codex_id {
+                return Err(SemanticError::CodexMismatch);
+            }
+            let api_id = api.api_id();
+            if !ids.insert(api_id) {
+                return Err(SemanticError::DuplicatedApi(api_id));
+            }
+        }
+
+        // Check codex libs for redundancies and completeness
+        let lib_map = self
+            .codex_libs
+            .iter()
+            .map(|lib| (lib.lib_id(), lib))
+            .collect::<IndexMap<_, _>>();
+
+        let mut lib_ids = codex
+            .verifiers
+            .values()
+            .map(|entry| entry.lib_id)
+            .collect::<IndexSet<_>>();
+        let mut i = 0usize;
+        let mut count = lib_ids.len();
+        while i < count {
+            let id = lib_ids.get_index(i).expect("index is valid");
+            let lib = lib_map.get(id).ok_or(SemanticError::MissedCodexLib(*id))?;
+            lib_ids.extend(lib.libs.iter().copied());
+            count = lib_ids.len();
+            i += 1;
+        }
+        for id in lib_map.keys() {
+            if !lib_ids.contains(id) {
+                return Err(SemanticError::ExcessiveCodexLib(*id));
+            }
+        }
+
+        // Check API libs for redundancies and completeness
+        let lib_map = self
+            .api_libs
+            .iter()
+            .map(|lib| (lib.lib_id(), lib))
+            .collect::<IndexMap<_, _>>();
+
+        let mut lib_ids = indexset![];
+        for api in self.apis() {
+            for agg in api.aggregators.values() {
+                if let Aggregator::AluVM(entry) = agg {
+                    lib_ids.insert(entry.lib_id);
+                }
+            }
+            for glob in api.global.values() {
+                if let StateConvertor::AluVM(entry) = glob.convertor {
+                    lib_ids.insert(entry.lib_id);
+                }
+                if let StateBuilder::AluVM(entry) = glob.builder {
+                    lib_ids.insert(entry.lib_id);
+                }
+                if let RawConvertor::AluVM(entry) = glob.raw_convertor {


In the Semantics::check() method, there are two separate code blocks checking libraries that follow a very similar pattern. This is a good candidate for refactoring into a common helper function to reduce duplication and improve readability.

ghost · 2025-06-01T18:16:15Z

 #[derive(StrictType, StrictDumb, StrictEncode)]
+// We must not derive or implement StrictDecode for Issuer, since we cannot validate signature
+// inside it
 #[strict_type(lib = LIB_NAME_SONIC)]
 pub struct Articles {
-    apis: ApiDescriptor,
+    /// We can't use [`Issuer`] here since we will duplicate the codex between it and the [`Issue`].
+    /// Thus, a dedicated substructure [`Semantics`] is introduced, which keeps a shared part of
+    /// both [`Issuer`] and [`Articles`].
+    semantics: Semantics,
+    /// Signature from the contract issuer (`issue.meta.issuer`) over the articles' id.
+    ///
+    /// NB: it must precede the issue, which contains genesis!
+    /// Since genesis is read with a stream-supporting procedure later.
+    sig: Option<SigBlob>,
+    /// The contract issue.
    issue: Issue,
 }

 impl Articles {
-    fn articles_id(&self) -> ArticlesId {
-        let custom_api_ids = SmallOrdMap::from_iter_checked(
-            self.apis
-                .custom
-                .iter()
-                .map(|(name, api)| (name.clone(), api.api_id())),
-        );
-        ArticlesCommitment {
-            contract_id: self.contract_id(),
-            default_api_id: self.apis.default.api_id(),
-            custom_api_ids,
+    /// Construct articles from a signed contract semantic and the contract issue under that
+    /// semantics.


The Articles::with constructor now takes a signature validator, which is an improvement for flexibility. However, the signature validator accepts a generic error type E that gets discarded when mapping to SemanticError::InvalidSignature. This loses specific error details. Consider using a more specific error type or preserving the original error for better debugging.

ghost · 2025-06-01T18:16:17Z

    pub fn genesis_opid(&self) -> Opid { self.issue.genesis_opid() }

-    pub fn apis(&self) -> &ApiDescriptor { &self.apis }
-    pub fn default_api(&self) -> &Api { &self.apis.default }
-    pub fn custom_apis(&self) -> impl Iterator<Item = (&TypeName, &Api)> { self.apis.custom.iter() }
-    pub fn types(&self) -> &TypeSystem { &self.apis.types }
+    /// Get a reference to the contract semantic.
+    pub fn semantics(&self) -> &Semantics { &self.semantics }
+    /// Get a reference to the default API.
+    pub fn default_api(&self) -> &Api { &self.semantics.default }
+    /// Get an iterator over the custom APIs.
+    pub fn custom_apis(&self) -> impl Iterator<Item = (&TypeName, &Api)> { self.semantics.custom.iter() }
+    /// Get a reference to the type system.
+    pub fn types(&self) -> &TypeSystem { &self.semantics.types }
+    /// Iterates over all APIs, including the default and the named ones.
+    pub fn apis(&self) -> impl Iterator<Item = &Api> { self.semantics.apis() }
+    /// Iterates over all codex libraries.
+    pub fn codex_libs(&self) -> impl Iterator<Item = &Lib> { self.semantics.codex_libs.iter() }



The Articles::upgrade_apis method now returns a boolean to indicate whether an upgrade occurred. This is good for informing clients, but the commit doesn't add usage examples. Consider adding documentation showing how clients should handle this return value.

dr-orlovsky added 3 commits May 23, 2025 20:56

add more state builders and adaptors

4b6d67a

api: restructure API versioning

6d8d6e5

api: overhaul articles/issuer invariance and sigs

fb38d71

api: remove sig_validator where it is not needed

2107203

dr-orlovsky force-pushed the v0.12 branch from 62b97a8 to 2107203 Compare May 25, 2025 11:47

dr-orlovsky added 10 commits May 25, 2025 14:09

api: check that the set of libs in semantics matches the codex requir…

e06284c

…ements

chore: fix clippy lints

ef5fef7

ledger: properly use Binfile with export/accept ops

6c05c46

api: remove unused issuer file constants

845e31a

api: add is_signed to Articles and Issuer

1f26c8d

fix: default build

388e198

api: reorg some aggregators

a488ac8

rename immutable to global and destructible to owned

4e84856

api: allow AluVM aggregators in the future

5c06d6e

api: aggregators overhaul

53e7c17

dr-orlovsky force-pushed the v0.12 branch from e02d4dc to 53e7c17 Compare May 25, 2025 19:13

dr-orlovsky added 5 commits May 25, 2025 21:56

ledger: add information about number of operations to the export

2ec5e8e

chore

072ee44

ledger: move position of op count in consignment to make it strict-co…

1b39256

…mpatible

ledger: add version to the consignment stream

4d516e7

ledger: add extension blocks to the consignment

6af7835

Signed-off-by: Dr Maxim Orlovsky <orlovsky@lnp-bp.org>

dr-orlovsky force-pushed the v0.12 branch from 6d9d5d9 to 6af7835 Compare May 26, 2025 18:27

dr-orlovsky added 5 commits May 26, 2025 21:08

api: reserve AluVM state builders, covertors and arithmetics

8336056

api: reserve AluVM variant in raw state convertors

9ba4233

api: check for API libraries

9fd1993

api: final touches to aggregators

e1dbe60

api: fix ccomitting to default API in API checksums

f948606

dr-orlovsky force-pushed the v0.12 branch from 7b6b164 to f948606 Compare May 26, 2025 21:01

ledger: add export_all_exu method

e6e71c9

dr-orlovsky added 2 commits May 27, 2025 13:10

api: fix serde yaml nested enums problem

d90a5f4

api: improve and test asset spec

e6df2aa

dr-orlovsky force-pushed the v0.12 branch from b9059fc to e6df2aa Compare May 27, 2025 11:45

dr-orlovsky added 10 commits May 27, 2025 19:05

api: add first and last aggregator

34070cd

test: api checksum

fc16587

api: add plans for aggregators

3d3c072

chore: update to use RCP-250528A

26c996a

api: add None aggregator

59e56b7

api: add more aggregators

2858685

test: new api aggregators

3e9636e

Merge pull request #25 from AluVM/feat/opwitness

73a5fd5

Bring changes from ultrasonic RC2

ledger: test export/accept operations

22748d3

test: issuer persistence

68e9320

dr-orlovsky force-pushed the v0.12 branch from 12fcbd4 to 68e9320 Compare June 1, 2025 16:24

test: API adapters

6f1306e

dr-orlovsky force-pushed the v0.12 branch from 5b25c18 to 6f1306e Compare June 1, 2025 17:52

test: better API aggregators test coverage

c74a154

ghost reviewed Jun 1, 2025

View reviewed changes

Comment thread api/src/builders.rs

dr-orlovsky merged commit 509c3a9 into master Jun 1, 2025
29 checks passed

dr-orlovsky mentioned this pull request Jun 1, 2025

Use APIs improvements from Sonic; support contract import RGB-WG/rgb-std#315

Merged


		//! API defines how a contract can be interfaced by software.
		//! API defines how software can interface a contract.

Uh oh!

Conversation

dr-orlovsky commented May 25, 2025

Uh oh!

codecov Bot commented May 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

dr-orlovsky commented Jun 1, 2025

Uh oh!

ghost commented Jun 1, 2025

Uh oh!

ghost commented Jun 1, 2025

PR Description

Key Technical Changes

Architecture Decisions

Dependencies and Interactions

Risk Considerations

Notable Implementation Details

Uh oh!

ghost Jun 1, 2025

Choose a reason for hiding this comment

Uh oh!

ghost Jun 1, 2025

Choose a reason for hiding this comment

Uh oh!

ghost Jun 1, 2025

Choose a reason for hiding this comment

Uh oh!

ghost Jun 1, 2025

Choose a reason for hiding this comment

Uh oh!

ghost Jun 1, 2025

Choose a reason for hiding this comment

Uh oh!

ghost Jun 1, 2025

Choose a reason for hiding this comment

Uh oh!

ghost Jun 1, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

codecov Bot commented May 25, 2025 •

edited

Loading