refactor: enforce tx input encryption on submission by juan518munoz · Pull Request #2381 · 0xMiden/node

juan518munoz · 2026-07-27T18:32:17Z

Summary

This PR makes sealing mandatory. New flow now implies clients calling GetTransactionEncryptionKey before submission to seal inputs.

Plain transaction_inputs is gone from both submission messages, replaced by:

message ProvenTransaction {
  bytes transaction = 1;
  SealedTransactionInputs sealed_transaction_inputs = 2;
}

message SealedTransactionInputs {
  bytes key_id = 1;      // which key this was sealed to
  bytes ciphertext = 2;  // serialized SealedMessage
}

message TransactionBatch {
  bytes batch_proof = 1;
  optional bytes proposed_batch = 2;
  repeated SealedTransactionInputs sealed_transaction_inputs = 3;
}

Changelog

[[entry]]
scope = "protocol"
impact = "breaking"
description = "Transaction submissions now carry their private inputs sealed. `ProvenTransaction.transaction_inputs` and `TransactionBatch.transaction_inputs` are replaced by `sealed_transaction_inputs`, a `SealedTransactionInputs` envelope holding the encryption key id and the ciphertext."

[[entry]]
scope = "rpc"
impact = "breaking"
description = "`SubmitProvenTx` and `SubmitProvenTxBatch` reject submissions whose transaction inputs are not sealed against the validator's transaction encryption key. Fetching that key with `GetTransactionEncryptionKey` is now a required first step before submitting."

[[entry]]
scope = "rpc"
impact = "fixed"
description = "The transaction and batch submission handlers no longer log the full request, which was writing clients' private transaction inputs into logs and traces."

[[entry]]
scope = "rpc"
impact = "changed"
description = "`GetTransactionEncryptionKey` responses are cached briefly, since every submitting client must now fetch the key and a validator's answer is fixed for its lifetime."

[[entry]]
scope = "validator"
impact = "changed"
description = "Submitted transaction inputs are unsealed before validation, binding each ciphertext to one encryption key, one network and one transaction."

[[entry]]
scope = "ntx-builder"
impact = "changed"
description = "Network transactions are submitted with sealed transaction inputs, and the cached encryption key is refreshed automatically if the validator reports an unknown key."

[[entry]]
scope = "network-monitor"
impact = "changed"
description = "Counter increments and account deployments are submitted with sealed transaction inputs."

igamigo

Leaving a couple of comments for now but the overall shape looks good

igamigo · 2026-07-27T19:13:33Z

+pub async fn create_genesis_aware_rpc_client_with_commitment(
+    rpc_url: &Url,
+    timeout: Duration,
+) -> Result<(RpcClient, Word)> {


Should this not just replace the previous function?

igamigo · 2026-07-27T19:26:53Z

+    let (pool, genesis_commitment) =
+        create_genesis_aware_rpc_client_pool(&rpc_url, Duration::from_secs(30), connections)
+            .await
+            .expect("failed to create RPC client pool");


Instead of the genesis commitment, I believe this could also just return the TransactionInputsSealer

igamigo · 2026-07-27T19:27:23Z

+/// Holding one avoids re-fetching the key per submission; callers should discard it when the
+/// validator reports an unknown key id, which is how a key change is detected.
+#[derive(Debug, Clone)]
+pub struct TransactionInputSealer {


nit: I'd name this TransactionInputsSealer instead

igamigo · 2026-07-27T19:30:31Z

+    scheme: u32,
+    key_id: Vec<u8>,
+    sealing_key: SealingKey,


Wonder if this should be its own domain type and then we could have an impl From<proto::transaction::TransactionEncryptionKey> for TransactionEncryptionKey

SantiagoPittella

Looking good

SantiagoPittella · 2026-07-27T21:05:55Z

+impl TransactionInputSealer {
+    /// Builds a sealer from a served encryption key and the genesis commitment of the network the
+    /// inputs will be submitted to.
+    pub fn new(


Should we use/verify attestation here?

SantiagoPittella · 2026-07-27T21:09:20Z

+    async fn call_submit_proven_transaction(
+        &self,
+        tx: &ProvenTransaction,
+        sealed: Option<proto::transaction::SealedTransactionInputs>,


Are we using the None-option somewhere? Now that it is enforced, could we ever need an Option here?

SantiagoPittella · 2026-07-27T21:14:30Z

        let original_accept_header = metadata.get(http::header::ACCEPT.as_str()).cloned();

-        tracing::trace!(target: LOG_TARGET, ?request);
+        tracing::trace!(target: LOG_TARGET, "Received transaction submission");


Should we add any extra information herE?

SantiagoPittella · 2026-07-27T21:17:27Z

+        let result = self.inner.clone().submit_proven_tx(request).await;
+
+        // A validator restarted with a different encryption key rejects with `failed_precondition`.
+        // Drop the cached key so the next attempt seals against the current one.
+        if let Err(status) = &result
+            && status.code() == tonic::Code::FailedPrecondition
+        {
+            self.invalidate_sealer().await;
+        }
+        result?;

        Ok(())


Suggested change

let result = self.inner.clone().submit_proven_tx(request).await;

// A validator restarted with a different encryption key rejects with `failed_precondition`.

// Drop the cached key so the next attempt seals against the current one.

if let Err(status) = &result

&& status.code() == tonic::Code::FailedPrecondition

{

self.invalidate_sealer().await;

}

result?;

Ok(())

match self.inner.clone().submit_proven_tx(request).await {

Ok(_) => Ok(()),

Err(status) => {

// A validator restarted with a different encryption key rejects with

// `failed_precondition`. Drop the cached key so the next attempt seals against the

// current one.

if status.code() == tonic::Code::FailedPrecondition {

self.invalidate_sealer().await;

}

Err(status)

},

}

huitseeker

Main item: let's check the attestation in TransactionInputSealer::new.

huitseeker · 2026-07-27T21:50:14Z


+        let mut cache = self.encryption_key_cache.lock().await;
+        if let Some(cached) = cache.as_ref()
+            && cached.fetched_at.elapsed() < ENCRYPTION_KEY_CACHE_TTL


Could we drop this cache, or invalidate it when a forwarded submission returns FailedPrecondition? After a validator restarts with a new key, the submit response tells the client to re-fetch, but this branch serves the same stale key for up to 30 seconds. The ntx builder then seals with the old key again and burns another note attempt instead of recovering. A restart test that changes the upstream key would cover this path.

Shouldn't these keys be very long lived, relatively speaking?

I imagine the clients will already be caching the key themselves, so imo we can simplify and just always forward the request.

huitseeker · 2026-07-27T21:50:21Z

+        // A validator restarted with a different encryption key rejects with `failed_precondition`.
+        // Drop the cached key so the next attempt seals against the current one.
+        if let Err(status) = &result
+            && status.code() == tonic::Code::FailedPrecondition


Could this retry once after invalidating the sealer? NtxContext::submit only retries statuses accepted by is_transient_status, and that function excludes FailedPrecondition. This error therefore escapes immediately, so the actor marks every note in the candidate failed before a later block can try the fresh key. Re-fetching and resealing once here would make the key-change path recover without penalizing the notes.

huitseeker · 2026-07-27T22:40:13Z

+impl TransactionInputSealer {
+    /// Builds a sealer from a served encryption key and the genesis commitment of the network the
+    /// inputs will be submitted to.
+    pub fn new(


Mirko-von-Leipzig · 2026-07-28T10:18:03Z

 [Service]
 Type=exec
 Environment="OTEL_SERVICE_NAME=miden-validator"
+EnvironmentFile=-/etc/opt/miden-validator/env


Why is this added?

Mirko-von-Leipzig · 2026-07-28T10:24:47Z

+  // Opaque identifier of the encryption key the inputs were sealed against, copied from
+  // [TransactionEncryptionKey.key_id].
+  bytes key_id = 1;
+
+  // The transaction inputs sealed with [miden_protocol::crypto::ies::SealingKey], encoded using
+  // the [miden_protocol::crypto::utils::Serializable] implementation for
+  // [miden_protocol::crypto::ies::SealedMessage].
+  bytes ciphertext = 2;


Question - will we only ever support a single key type? Or should this be an enum?

Mirko-von-Leipzig · 2026-07-28T10:30:43Z

+nodes without one forward it to the upstream RPC source. Responses are cached briefly, since every submitting client
+must fetch the key first and a validator's answer does not change while it is running.


Should keys not have an expiry set? At the moment this expiry is simply forever, so I don't know we would cache things briefly only. I'll revisit this when I see the impl.

Mirko-von-Leipzig · 2026-07-28T10:39:35Z

        let original_accept_header = metadata.get(http::header::ACCEPT.as_str()).cloned();

-        tracing::trace!(target: LOG_TARGET, ?request);
+        tracing::trace!(target: LOG_TARGET, batch_size = request.sealed_transaction_inputs.len());


Suggested change

tracing::trace!(target: LOG_TARGET, batch_size = request.sealed_transaction_inputs.len());

tracing::trace!(target: LOG_TARGET, batch.size = request.sealed_transaction_inputs.len());

Mirko-von-Leipzig · 2026-07-28T10:42:42Z


+        let mut cache = self.encryption_key_cache.lock().await;
+        if let Some(cached) = cache.as_ref()
+            && cached.fetched_at.elapsed() < ENCRYPTION_KEY_CACHE_TTL


Shouldn't these keys be very long lived, relatively speaking?

I imagine the clients will already be caching the key themselves, so imo we can simplify and just always forward the request.

Mirko-von-Leipzig · 2026-07-28T10:50:04Z

+    /// Genesis commitment of the network being submitted to, bound into the associated data of
+    /// sealed transaction inputs.
+    genesis_commitment: Word,
+    /// Cached sealer for transaction inputs, fetched on first submission.
+    sealer: Arc<RwLock<Option<TransactionInputSealer>>>,


Same here. There is no reason this can't already be populated, or handled by the client.

Mirko-von-Leipzig · 2026-07-28T10:51:50Z

+        spawn_blocking_in_current_span(move || {
+            UnsealingKey::X25519XChaCha20Poly1305(secret_key)
+                .unseal_bytes_with_associated_data(message, &associated_data)
+                .context("AEAD authentication failed")
+        })
+        .await


Why are these functions async?

Mirko-von-Leipzig · 2026-07-28T10:55:15Z

            scheme: Self::scheme_id(),
            key_id: self.key_id(),


Should these not be an enum? We surely don't support an open-ended set of schemas?

Mirko-von-Leipzig · 2026-07-28T11:19:58Z

+pub struct TransactionInputSealer {
+    scheme: u32,
+    key_id: Vec<u8>,
+    sealing_key: SealingKey,
+    genesis_commitment: Word,
+}


Are we sure this shouldn't be an enum?

Mirko-von-Leipzig · 2026-07-28T11:22:11Z

+pub fn transaction_inputs_associated_data(
+    scheme: u32,
+    key_id: &[u8],
+    genesis_commitment: Word,
+    tx_id: TransactionId,
+) -> Vec<u8> {


Should this be pub? Where else is it used? I assume that's why it isn't:

Suggested change

pub fn transaction_inputs_associated_data(

scheme: u32,

key_id: &[u8],

genesis_commitment: Word,

tx_id: TransactionId,

) -> Vec<u8> {

impl TransactionInputSealer {

fn build_associated_data(&self, tx_id: TransactionId) -> Vec<u8> {

...

}

}

juan518munoz · 2026-07-29T19:29:01Z

Closing, superseeded by #2389

refactor: force tx input encryption on submission

8427c13

igamigo requested a review from SantiagoPittella July 27, 2026 19:17

igamigo reviewed Jul 27, 2026

View reviewed changes

SantiagoPittella reviewed Jul 27, 2026

View reviewed changes

huitseeker requested review from Mirko-von-Leipzig and huitseeker July 27, 2026 21:28

huitseeker mentioned this pull request Jul 27, 2026

Store validated sealed transaction inputs #2385

Closed

huitseeker reviewed Jul 27, 2026

View reviewed changes

Mirko-von-Leipzig reviewed Jul 28, 2026

View reviewed changes

huitseeker mentioned this pull request Jul 28, 2026

Enforce encrypted transaction inputs on submission #2389

Merged

juan518munoz closed this Jul 29, 2026

		nodes without one forward it to the upstream RPC source. Responses are cached briefly, since every submitting client
		must fetch the key first and a validator's answer does not change while it is running.

	tracing::trace!(target: LOG_TARGET, batch_size = request.sealed_transaction_inputs.len());
	tracing::trace!(target: LOG_TARGET, batch.size = request.sealed_transaction_inputs.len());

Uh oh!

Conversation

juan518munoz commented Jul 27, 2026

Summary

Changelog

Uh oh!

igamigo 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!

SantiagoPittella 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!

huitseeker 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!

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!

juan518munoz commented Jul 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants