refactor: extract free functions for enum and numeric oracle event operations #130

rorp · 2025-10-09T23:08:21Z

Summary

This PR extracts the core oracle event creation and signing logic into standalone free functions, improving code reusability and testability while maintaining the existing Oracle API.

Major Changes

Function Extraction

create_enum_event() - Standalone function for creating enum event announcements
sign_enum_event() - Standalone function for signing enum event outcomes
create_numeric_event() - Standalone function for creating numeric event announcements
sign_numeric_event() - Standalone function for signing numeric event outcomes with clamping logic
Helper functions: get_min_value(), get_max_value(), derive_signing_key()

Architectural Benefits

Code Reusability: Core DLC logic can be used independently of the Oracle struct
Testability: Free functions are easier to unit test in isolation
Separation of Concerns: DLC oracle logic separated from storage/persistence concerns
API Flexibility: Users can choose between high-level Oracle methods or low-level functions

Implementation Details

Oracle methods now delegate to the extracted free functions
Maintains backward compatibility - all existing Oracle API remains unchanged
Enhanced error handling with proper validation in free functions
Comprehensive input validation for edge cases (empty strings, invalid ranges, etc.)

Minor Improvements

Documentation: Added comprehensive Rust docs for all public functions
Unit Tests: Added focused tests for free functions
DLC Spec References: Linked to relevant DLC specification sections

Impact

No Breaking Changes: All existing code continues to work unchanged
Enhanced Flexibility: Developers can now use either high-level or low-level APIs
Better Testing: Isolated unit tests for core cryptographic operations
Improved Maintainability: Clear separation between DLC logic and infrastructure

Testing

All existing tests pass without modification
New unit tests cover free functions and edge cases
Clamping behavior thoroughly tested for both signed and unsigned numeric events
This refactor significantly improves the codebase architecture while maintaining full backward compatibility, making the oracle system more modular and testable.

kwsantiago · 2025-10-17T23:09:39Z

Take a look at #104 in case there's anything you can grab from there. I can close 104, pending @bennyhodl

kwsantiago

Good code quality, excellent tests, solid documentation. Left some comments for silent behavior change from errors to clamping.

kormir/src/lib.rs

kwsantiago · 2025-10-17T23:40:20Z

kormir/src/lib.rs

+    if event_id.is_empty() {
+        return Err(Error::InvalidArgument);
+    }
+    if base != 2 {


Base 2 is hardcoded but the function accepts a base parameter.

Either remove the parameter or support other bases.

I'm planning to add support for other bases in a separate PR.

kwsantiago · 2025-10-17T23:41:10Z

kormir/src/lib.rs

+    if base != 2 {
+        return Err(Error::InvalidArgument);
+    }
+    if num_digits == 0 || num_digits > 63 {


Why 63?

This should be a constant with an explanation of why this number was chosen.

Because we use i64 as outcome type which uses 63 bits for the value and 1 bit for the sign. Probably it makes sense to use BigInt and BigUint here. But again, this should be done in a separate PR.

kwsantiago · 2025-10-17T23:43:32Z

kormir/src/lib.rs

+    );
+
+    // verify our nonce is the same as the one in the announcement
+    debug_assert!(sig.encode()[..32] == announcement.oracle_event.oracle_nonces[0].serialize());


The old code used an iterator while the new code is collecting all nonce keys which could lead to a small performance regression for large nonce sets.

I don't think we need an iterator to collect 1 nonce.

We should at least check that we have a nonce. Validating the announcement will check that we have 1 nonce.

In fact we don't need this line at all. It was added to check if rust-dlc signing algorithm worked as expected. And it did.

Moved these checks to the unit tests.

kwsantiago · 2025-10-17T23:48:06Z

kormir/src/lib.rs

-            oracle_nonces,
-            event_id,
+        let nonce_indexes = self.storage.get_next_nonce_indexes(1).await?;
+        if nonce_indexes.len() != 1 {


This check seems defensive but the free function doesn't do it. There is inconsistent validation between Oracle methods and free functions.

Of course the free function doesn't do it. The compiler checks it. This was not an inconsistency, but a bug in kormir, which could lead to unexpected panics.

kwsantiago · 2025-10-17T23:48:31Z

kormir/src/lib.rs

+/// * [Oracle Announcement](https://github.com/discreetlogcontracts/dlcspecs/blob/master/Messaging.md#the-oracle_announcement-type)
+/// * [Oracle Event](https://github.com/discreetlogcontracts/dlcspecs/blob/master/Messaging.md#the-oracle_event-type)
+/// * [Digit Decomposition Event Descriptor](https://github.com/discreetlogcontracts/dlcspecs/blob/master/Messaging.md#digit_decomposition_event_descriptor)
+#[allow(clippy::too_many_arguments)]


Nit: consider a builder pattern or config struct instead of suppressing the lint.

Good idea for a separate PR.

rorp · 2025-11-24T20:08:28Z

Take a look at #104 in case there's anything you can grab from there. I can close 104, pending @bennyhodl

This PR and #104 are orthogonal.

#104 creates CLI commands that allow users to connect to a Kormir server instance in order for the server to create announcements and attestations.

This PR extracts the core logic of creating announcements and attestations so it can be used in other orcale apps. Imagine a weather web-site, that posts announcements on Twitter; attests them with the data from their internal database and again posts attestations on Twitter; and uses their custom private key setup without exposing sign- endpoints. This PR makes it easier for them to build this kind of app.

bennyhodl

thanks 🤙🏼

small nits

kormir/src/lib.rs

bennyhodl · 2025-11-25T18:50:12Z

kormir/src/lib.rs

+    );
+
+    // verify our nonce is the same as the one in the announcement
+    debug_assert!(sig.encode()[..32] == announcement.oracle_event.oracle_nonces[0].serialize());


We should at least check that we have a nonce. Validating the announcement will check that we have 1 nonce.

bennyhodl · 2025-11-25T19:06:22Z

kormir/src/lib.rs

+        return Err(Error::InvalidArgument);
+    }
+    if key_pair.x_only_public_key().0 != announcement.oracle_public_key {
+        return Err(Error::InvalidAnnouncement);


InvalidAnnouncement might be the wrong error here. The signature can still be valid for that public key

We don't want to attest someone else's announcements. This check validates if it's the case.

kormir/src/lib.rs

kormir/src/error.rs

kormir/src/lib.rs

kwsantiago · 2025-11-28T16:21:49Z

kormir/src/lib.rs

+    // verify our signature
+    if secp
+        .verify_schnorr(&sig, &msg, &key_pair.x_only_public_key().0)
+        .is_err()
+    {
+        return Err(Error::Internal);
+    };


The old code verified that the nonce in the signature matched the announcement's nonce.

Should we add a runtime check to ensure the caller passed the correct nonce key matching the announcement?

Good catch! I added an explicit check for the nonces and nonce keys.

kwsantiago · 2025-11-28T16:28:33Z

kormir/src/lib.rs

+            &data.announcement,
+            outcome,
+            &nonce_keys,
+            false,


Shouldn't clamp_outcome be exposed as an optional param instead of hard-coding to false? If it's always set to false then the Oracle struct can never use clamping.

It shouldn't be there in the first place. The oracle must be spec compliant. Reverted it to the initial commit.

kormir/src/lib.rs

kwsantiago · 2025-11-28T17:15:25Z

kormir/src/error.rs

 /// Kormir error type
 #[derive(Debug, Clone)]
 pub enum Error {
-    /// Invalid argument given


Since InvalidArgument was removed, this is a breaking change for anyone using Error::InvalidArgument

May want to add in changelog. The new errors you added are better though.

Reverted it and made it deprecated. Will be removed in the next minor release.

kormir/src/lib.rs

…erations

rorp force-pushed the refactor_kormir branch 2 times, most recently from 8031aa2 to 26ca2d8 Compare October 9, 2025 23:32

kwsantiago suggested changes Oct 17, 2025

View reviewed changes

rorp force-pushed the refactor_kormir branch from 26ca2d8 to 7f19169 Compare November 3, 2025 22:27

rorp force-pushed the refactor_kormir branch from a9795e5 to 57df97d Compare November 24, 2025 19:57

rorp force-pushed the refactor_kormir branch 2 times, most recently from 0cf7156 to a7c14a9 Compare November 25, 2025 02:18

rorp requested a review from kwsantiago November 25, 2025 02:25

bennyhodl requested changes Nov 25, 2025

View reviewed changes

kwsantiago suggested changes Nov 25, 2025

View reviewed changes

rorp force-pushed the refactor_kormir branch from a7c14a9 to 00f83b7 Compare November 27, 2025 23:14

rorp requested review from bennyhodl and kwsantiago November 27, 2025 23:17

rorp force-pushed the refactor_kormir branch from 00f83b7 to 75a2b5d Compare November 27, 2025 23:25

kwsantiago suggested changes Nov 28, 2025

View reviewed changes

refactor: extract free functions for enum and numeric oracle event op…

cfc507c

…erations

rorp force-pushed the refactor_kormir branch from 75a2b5d to cfc507c Compare November 30, 2025 23:52

refactor: extract free functions for enum and numeric oracle event operations #130

Are you sure you want to change the base?

refactor: extract free functions for enum and numeric oracle event operations #130

Uh oh!

Conversation

rorp commented Oct 9, 2025

Summary

Major Changes

Function Extraction

Architectural Benefits

Implementation Details

Minor Improvements

Impact

Testing

Uh oh!

kwsantiago commented Oct 17, 2025

Uh oh!

kwsantiago left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rorp Nov 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rorp commented Nov 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bennyhodl left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rorp Nov 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

rorp Nov 24, 2025 •

edited

Loading

rorp commented Nov 24, 2025 •

edited

Loading

rorp Nov 30, 2025 •

edited

Loading