Skip to content

feat: cbor witnesses support #830

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mduthey wants to merge 2 commits into
base: main
Choose a base branch
from
Open

feat: cbor witnesses support #830

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f7204fc
feat: add support for cbor witnesses
mduthey Dec 31, 2025
a041073
chore: use SubmitParams and WitnessInput from tx3_resolver
mduthey Jan 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 28 additions & 10 deletions crates/trp/src/methods.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
use jsonrpsee::types::Params;
use pallas::{codec::utils::NonEmptySet, ledger::primitives::conway::VKeyWitness};
use pallas::{
codec::utils::NonEmptySet,
ledger::primitives::conway::{VKeyWitness, WitnessSet},
};
use std::sync::Arc;

use tx3_resolver::trp::{ResolveParams, SubmitParams, SubmitResponse, SubmitWitness, TxEnvelope};
use tx3_resolver::{trp::{ResolveParams, SubmitResponse, SubmitParams, WitnessInput, TxEnvelope}};

Check failure on line 8 in crates/trp/src/methods.rs

View workflow job for this annotation

GitHub Actions / Check Build 

unresolved import `tx3_resolver::trp::WitnessInput`

use dolos_core::{facade::receive_tx, Domain, MempoolAwareUtxoStore, StateStore as _};

Expand Down Expand Up @@ -39,15 +42,32 @@
})
}

fn apply_witnesses(original: &[u8], witnesses: &[SubmitWitness]) -> Result<Vec<u8>, Error> {
fn apply_witnesses(original: &[u8], witnesses: &[WitnessInput]) -> Result<Vec<u8>, Error> {
let tx = pallas::ledger::traverse::MultiEraTx::decode(original)?;

let mut tx = tx.as_conway().ok_or(Error::UnsupportedTxEra)?.to_owned();

let map_witness = |witness: &SubmitWitness| VKeyWitness {
vkey: Vec::<u8>::from(witness.key.clone()).into(),
signature: Vec::<u8>::from(witness.signature.clone()).into(),
};
let mut new_vkeys = Vec::new();

for witness in witnesses {
match witness {
WitnessInput::Object(w) => {
new_vkeys.push(VKeyWitness {
vkey: Vec::<u8>::from(w.key.clone()).into(),
signature: Vec::<u8>::from(w.signature.clone()).into(),
});
}
WitnessInput::Hex(h) => {
let bytes = hex::decode(h).map_err(|_| Error::InternalError("Invalid witness hex".into()))?;
let witness_set: WitnessSet = pallas::codec::minicbor::decode(&bytes)
.map_err(|_| Error::InternalError("Invalid witness set cbor".into()))?;

if let Some(vkeys) = witness_set.vkeywitness {
new_vkeys.extend(vkeys.to_vec());
}
}
}
}
Comment on lines +50 to +70
Copy link

@coderabbitai coderabbitai bot Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Consider adding size limits and validation for witness inputs.

The function accepts unbounded hex-encoded witness data without size validation, which could be exploited for denial-of-service attacks through excessive memory consumption or maliciously crafted CBOR payloads.

Consider adding:

  • Maximum size limits for hex-encoded witness data
  • Validation of the decoded witness set structure
  • Limits on the total number of witnesses being added
#!/bin/bash
# Description: Check if there are any existing size limits or validation patterns in the codebase

echo "=== Checking for existing validation patterns ==="
rg -n "max.*size|size.*limit|validate.*witness" --type rust -C3

echo ""
echo "=== Checking error handling patterns ==="
rg -n "InternalError|Error::" crates/trp/src -C2
🤖 Prompt for AI Agents 
In @crates/trp/src/methods.rs around lines 50-70, The loop that decodes witness
hex (involving WitnessInput::Hex, hex::decode, pallas::codec::minicbor::decode,
and pushing into new_vkeys) lacks size and structural validation; add constants
like MAX_WITNESS_HEX_BYTES and MAX_TOTAL_WITNESSES, check the incoming hex
string length before hex::decode and return a clear Error::InternalError (or a
new Error::ValidationError) if it exceeds the limit, validate the decoded CBOR
bytes length before minicbor::decode, ensure the decoded WitnessSet.vkeywitness
exists and its length is within per-message and aggregate limits before
extending new_vkeys, and enforce that new_vkeys.len() never exceeds
MAX_TOTAL_WITNESSES (returning an error if it would).


let mut witness_set = tx.transaction_witness_set.unwrap();
Copy link

@coderabbitai coderabbitai bot Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Handle missing witness set to prevent panic.

The unwrap() will panic if the transaction doesn't have an existing witness set. Transactions without witnesses are valid in Cardano, and this function should handle that case by initializing an empty witness set.

🔎 Proposed fix to handle missing witness set 
-    let mut witness_set = tx.transaction_witness_set.unwrap();
+    let mut witness_set = tx.transaction_witness_set.unwrap_or_else(|| WitnessSet {
+        vkeywitness: None,
+        native_script: None,
+        bootstrap_witness: None,
+        plutus_v1_script: None,
+        plutus_data: None,
+        redeemer: None,
+        plutus_v2_script: None,
+        plutus_v3_script: None,
+    });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
let mut witness_set = tx.transaction_witness_set.unwrap();
let mut witness_set = tx.transaction_witness_set.unwrap_or_else(|| WitnessSet {
vkeywitness: None,
native_script: None,
bootstrap_witness: None,
plutus_v1_script: None,
plutus_data: None,
redeemer: None,
plutus_v2_script: None,
plutus_v3_script: None,
});
🤖 Prompt for AI Agents 
In @crates/trp/src/methods.rs around line 72, The code currently calls
tx.transaction_witness_set.unwrap() which will panic when the transaction has no
witnesses; instead handle the None case by initializing an empty witness set
(e.g., create a new TransactionWitnessSet) and assign it to let mut witness_set
so the rest of the function can proceed safely; replace the unwrap with a match
or .unwrap_or_else(...) that constructs an empty witness set when
tx.transaction_witness_set is None and keep downstream uses of witness_set
unchanged.


Expand All @@ -57,9 +77,7 @@
.flat_map(|x| x.iter())
.cloned();

let new = witnesses.iter().map(map_witness);

let all: Vec<_> = old.chain(new).collect();
let all: Vec<_> = old.chain(new_vkeys).collect();

witness_set.vkeywitness = NonEmptySet::from_vec(all);

Expand Down
Loading