refactor!: bundle private/public i/o into action structs by agureev · Pull Request #604 · logos-blockchain/logos-execution-zone

agureev · 2026-07-08T18:03:40Z

🎯 Purpose

This refactors the message and the PPC output structs, enforcing the bijection between pre and post-states on the struct level. Before, e.g. having separate vectors for new_commitments and new_nullifiers would require being careful about their length mismatch.

This refactor reduces the mismatch possibilities by making sure that all pre and post state fields for public and private accounts come paired in an appropriate struct. E.g. the privacy-preserving circuit output struct becomes:

pub struct PrivacyPreservingCircuitOutput {
    pub public_actions: Vec<PublicAction>,
    pub private_actions: Vec<PrivateAction>,
    pub block_validity_window: BlockValidityWindow,
    pub timestamp_validity_window: TimestampValidityWindow,
}

The name Actions is taken from ZCash as a stand-in, feel free to suggest better naming.

⚙️ Approach

Define private and public action structs
Change the structs utilizing any paired vectors, i.e. Message and Output
Change usepoints of said structs
Refactor other parts of the codebase that could benefit from the change

🧪 How to Test

RISC0_DEV_MODE=1 cargo nextest run --workspace --exclude integration_tests --all-features --no-fail-fast

📋 PR Completion Checklist

Mark only completed items. A complete PR should have all boxes ticked.

Complete PR description
Implement the core functionality
Add/update tests
Add/update documentation and inline comments

Arjentix

Like the approach, much cleaner now.

Left some minor comments, will wait for undrafting and then will approve

Arjentix · 2026-07-08T19:06:31Z

+pub struct PublicActionWithOwner {
+    pub account_id: AccountId,
+    pub post_state: Account,
+}


Name of the struct does not reflect what's inside I think

Renamed to PublicActionWIthID, agreed that Owner is a vague term with program ownership alongside authorization

Arjentix · 2026-07-08T19:13:33Z

+            public_account_ids: public_actions.iter().map(|a| a.account_id.into()).collect(),
            nonces: nonces.iter().map(|x| x.0).collect(),
-            public_post_states: public_post_states.into_iter().map(Into::into).collect(),
-            encrypted_private_post_states: encrypted_private_post_states
-                .into_iter()
-                .map(Into::into)
+            public_post_states: public_actions.into_iter().map(|a| a.post_state.into()).collect(),
+            encrypted_private_post_states: private_actions
+                .iter()
+                .map(|a| a.encrypted_post_state.clone().into())
                .collect(),
-            new_commitments: new_commitments.into_iter().map(Into::into).collect(),
-            new_nullifiers: new_nullifiers
+            new_commitments: private_actions.iter().map(|a| a.commitment.clone().into()).collect(),
+            new_nullifiers: private_actions


I think It's worth changing this whole struct the same way you changed the core struct.

In case of any problems on Explorer side you can always use Claude, haha

should be better now! the message format has been changed accordingly!

github-actions · 2026-07-16T16:26:22Z

Benchmark for `d017a1f`

Click to view benchmark

Test	Base	PR	%
encryption/decrypt	553.7±4.46ns	553.3±4.41ns	-0.07%
encryption/encrypt	744.0±3.59ns	741.9±1.97ns	-0.28%
keychain/new_mnemonic	4.2±0.01ms	4.2±0.01ms	0.00%
keychain/new_os_random	4.2±0.08ms	4.2±0.02ms	0.00%
shared_secret_key/sender_encapsulate	49.4±5.33µs	50.8±1.66µs	+2.83%

moudyellaz

Left two comments. The three failing CI jobs look like infra (archive extraction + GitHub rate limit fetching risc0), probably just need a rerun.

moudyellaz · 2026-07-17T17:09:05Z

+pub struct PrivateAction {
+    pub nullifier: Nullifier,
+    pub root: CommitmentSetDigest,
+    pub commitment: Commitment,


One thing worth guarding here: after obfuscate_output_ordering, this commitment no longer belongs to this action's nullifier/encrypted_post_state (that's #561's unlinking doing its job), but the struct shape suggests they're paired, and the indexer protocol mirrors it downstream. Could commitments move out to a separate Vec<Commitment>, since nullifier/root/note are truly paired and commitments are an independent set? If you'd rather keep the bundling, a doc note on this field at both definition sites would save a future consumer from re-linking them.

I think I would rather keep the binding, otherwise this reintroduces the issue of length-matching.

But I agree that a doc addition would be nice!

Should be addressed in 0e957b5

Let me know if there are any other places you want me to document this

moudyellaz · 2026-07-17T17:09:05Z

 pub struct Message {
-    pub public_account_ids: Vec<AccountId>,
+    pub public_actions: Vec<PublicActionWithID>,
    pub nonces: Vec<Nonce>,


nonces is the one parallel vector left after this refactor, positionally paired with public_actions. Could nonce live inside PublicActionWithID, or does the signing/serialization format constrain it?

I agree that it would be nice to pair these! Indeed, the signing format currently seems to narrow it as the nonces for signature-checking are gotten by position from the nonces vector. There is probably some nice way to refactor this as well, but it seemed to be something to be undertaken in parallel, I concentrated only on the pre-post state pairing here.

Filed #655 to keep track of this

BREAKING! Before: The message format for private transactions included separate fields for private transaction state identifiers such as commitments and nullifiers. After: The commitments/nullifiers/ciphertexts are bound in an Action struct, likewise for public state-bearing messages. Mitigation: assume incoming messages are in a newly-specified format.

Pravdyvy

lgtm

agureev self-assigned this Jul 8, 2026

agureev added the priority:low Low priority label Jul 8, 2026

Arjentix reviewed Jul 8, 2026

View reviewed changes

agureev force-pushed the artem/bundle-actions branch from f3d58b4 to ed6349e Compare July 16, 2026 16:16

agureev changed the base branch from dev to artem/obfuscate-note-order July 16, 2026 16:17

agureev marked this pull request as ready for review July 16, 2026 16:52

moudyellaz reviewed Jul 17, 2026

View reviewed changes

agureev mentioned this pull request Jul 17, 2026

feat!: ciphertext padding #630

Open

7 tasks

agureev force-pushed the artem/obfuscate-note-order branch 2 times, most recently from 8539fb9 to b5d5d86 Compare July 28, 2026 12:14

agureev added 4 commits July 29, 2026 18:16

refactor: bundle private/public i/o into action struccts

f16c3bd

refactor(verifier): consume bundled public_actions directly

75326d0

chore: regenerate fixtures

846e194

agureev force-pushed the artem/bundle-actions branch from 9a2c9a2 to 846e194 Compare July 29, 2026 18:54

Pravdyvy approved these changes Jul 30, 2026

View reviewed changes

agureev requested a review from Arjentix July 30, 2026 13:31

agureev changed the base branch from artem/obfuscate-note-order to dev July 30, 2026 18:53

Merge remote-tracking branch 'origin/dev' into artem/bundle-actions

dbcdbb2

agureev mentioned this pull request Jul 30, 2026

Research Bundling Signatures with Nonce Data #655

Open

agureev added 2 commits July 31, 2026 00:47

chore: docs for PrivateAction commitment field

0e957b5

Merge remote-tracking branch 'origin/dev' into artem/bundle-actions

b03cef0

agureev mentioned this pull request Jul 31, 2026

refactor(lee): refactor private kinds #660

Open

7 tasks

agureev added priority:high High priority and removed priority:low Low priority labels Jul 31, 2026

Uh oh!

Conversation

agureev commented Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🎯 Purpose

⚙️ Approach

🧪 How to Test

📋 PR Completion Checklist

Uh oh!

Arjentix left a comment

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!

github-actions Bot commented Jul 16, 2026

Benchmark for d017a1f

Uh oh!

moudyellaz left a comment

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!

Pravdyvy left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

agureev commented Jul 8, 2026 •

edited

Loading

Benchmark for `d017a1f`