feat(template_lib): adds engine schnorr signature verification - #1574
Conversation
WalkthroughIntroduces domain-separated signature verification across the engine/runtime, adds a SignatureInvoke engine op and runtime event hooks, and charges a new SignatureVerification fee. Adds network-based fee table selection, updates imports, extends template libraries with signature types/args and verification helpers, and adds tests and templates to validate the new flow. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Template as Template (WASM)
participant ABI as EngineOp::SignatureInvoke
participant Runtime as RuntimeInterface
participant Verif as Signature Verifier (common_types)
participant Modules as Runtime Modules
participant Fees as FeeModule
Template->>ABI: call_engine(SignatureInvoke{Verify, args})
ABI->>Runtime: signature_invoke(Verify, EngineArgs)
Note over Runtime: Extract domain, msg, public_key, payload
Runtime->>Verif: payload.get_verifier().verify(domain, msg, pk, payload)
Verif-->>Runtime: bool (valid/invalid)
Runtime->>Modules: on_runtime_event(SignatureVerified)
Modules->>Fees: on_runtime_event(SignatureVerified)
Fees->>Fees: charge FeeSource::SignatureVerification
Runtime-->>ABI: InvokeResult(bool)
ABI-->>Template: decode(bool)
Note over Template: Proceed based on verification result
sequenceDiagram
autonumber
participant Node as Validator/Indexer
participant Util as app_utilities::fee_tables
participant Engine as TariTransactionProcessor
Node->>Util: get_fee_table_by_network(network)
Util-->>Node: &FeeTable (const)
Node->>Engine: new(config, template_manager, fee_table.clone())
Engine-->>Node: processor initialized
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
b6228b2 to
6cb524b
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (34)
Cargo.toml (2)
64-65: Duplicate workspace member entry.
utilities/transaction_submitterappears twice; remove the duplicate to avoid confusion.- "utilities/transaction_submitter", - "utilities/transaction_submitter", + "utilities/transaction_submitter",
181-181: Scope thederRC dependency to only crates that use it or pin to a stable release.Only crates/common_types references
der(crates/common_types/src/der_signature.rs). Removeder = "0.8.0-rc.9"from the workspace Cargo.toml (line 181) and add it to crates/common_types/Cargo.toml dependencies, or replace it with a stable release if available.crates/engine/src/fees/fee_table.rs (1)
11-11: New fee field: verify semver exposure; consider const ctor and docs
- Adding a public field to a public struct can break downstream code that constructs
FeeTablewith struct literals. Please confirm no external crates rely on this. If they do, consider adding a constructor/builder (and guiding users to it) before future field additions.- Minor:
zero_ratedcan beconst fn.- Suggest adding brief docs to the new field/getter.
Apply docs/const tweaks:
pub struct FeeTable { pub per_transaction_weight_cost: u64, pub per_module_call_cost: u64, pub per_byte_storage_cost: u64, pub per_event_cost: u64, pub per_log_cost: u64, - pub per_signature_verification_cost: u64, + /// Flat fee charged per successful signature verification event. + pub per_signature_verification_cost: u64, } impl FeeTable { - pub fn zero_rated() -> Self { + pub const fn zero_rated() -> Self { Self { per_transaction_weight_cost: 0, per_module_call_cost: 0, per_byte_storage_cost: 0, per_event_cost: 0, per_log_cost: 0, per_signature_verification_cost: 0, } } @@ - pub fn per_signature_verification_cost(&self) -> u64 { + /// Returns the flat fee charged per successful signature verification. + pub fn per_signature_verification_cost(&self) -> u64 { self.per_signature_verification_cost }Also applies to: 22-22, 46-48
crates/template_test_tooling/src/template_test.rs (1)
167-168: Test plumbing: assert signature fee is chargedGood to seed
per_signature_verification_cost: 1. Please add/extend a test that performs a signature verification and asserts the fee receipt includesFeeSource::SignatureVerificationwith the expected count×1, to catch regressions in the event hook.crates/template_lib/src/models/stealth.rs (1)
77-85: Accessors LGTMSmall, clear helpers; no behavior change. Consider adding brief rustdoc comments for TS/SDK consumers.
crates/template_lib/src/lib.rs (1)
49-50: Prefer macros 2.0 imports over crate-wide #[macro_use]Using
#[macro_use]at crate scope can cause name collisions. If feasible, export macros with macros 2.0 andusethem explicitly where needed (or re-export via prelude).Example (if macros are macros 2.0 compatible):
-#[macro_use] -pub mod models; +pub mod models; +// Then import/re-export specific macros where required: +// pub use crate::models::custom_signature_domain;crates/engine/src/wasm/process.rs (1)
54-55: Add a debug trace for SignatureInvoke (mirrors ProofInvoke).Helpful for diagnosing template calls; matches the existing pattern used for proofs.
- EngineOp::SignatureInvoke => Self::handle(store, env_mut, arg, |env, arg: SignatureInvokeArg| { - env.interface().signature_invoke(arg.action, arg.args.into()) - }), + EngineOp::SignatureInvoke => Self::handle(store, env_mut, arg, |env, arg: SignatureInvokeArg| { + log::debug!(target: LOG_TARGET, "signature action = {:?}", arg.action); + env.interface().signature_invoke(arg.action, arg.args.into()) + }),If verbosity is a concern, gate this behind the existing log level checks or feature flags.
Also applies to: 213-215
crates/common_types/src/lib.rs (1)
10-10: Consider narrowing re-exports to a stable surface.
pub use der_signature::*;andpub use engine_signature::*;expose all internals and may expand the public API more than intended. Re-export only the intended types/traits to keep semver surface tight.Also applies to: 12-12, 40-42
crates/engine/src/fees/fee_module.rs (1)
71-73: Rename param to ‘event’ for clarity.Minor readability nit:
call→event.Apply this diff:
- fn on_runtime_event(&self, track: &StateTracker, call: &RuntimeEvent) -> Result<(), RuntimeModuleError> { - match call { + fn on_runtime_event(&self, track: &StateTracker, event: &RuntimeEvent) -> Result<(), RuntimeModuleError> { + match event {crates/engine/src/runtime/module.rs (2)
19-21: Param naming nit: use_eventinstead of_call.Keeps terminology consistent across the codebase.
Apply this diff:
- fn on_runtime_event(&self, _track: &StateTracker, _call: &RuntimeEvent) -> Result<(), RuntimeModuleError> { + fn on_runtime_event(&self, _track: &StateTracker, _event: &RuntimeEvent) -> Result<(), RuntimeModuleError> {
24-27: ConsiderCopyfor small event enum.Deriving
Copyavoids incidental clones and matches pass‑by‑value usage patterns.Apply this diff:
-#[derive(Debug, Clone)] +#[derive(Debug, Clone, Copy)] pub enum RuntimeEvent { SignatureVerified, }applications/tari_indexer/src/dry_run/processor.rs (1)
87-90: Avoid unnecessary.clone()on&'static FeeTable(if ctor accepts a reference).
get_fee_table_by_networkreturns&'static FeeTable; cloning the reference is redundant ifTariTransactionProcessor::newaccepts&FeeTable.Apply this diff (if the constructor takes a reference):
- let payload_processor = - TariTransactionProcessor::new(self.config.clone(), self.template_manager.clone(), fee_table.clone()); + let payload_processor = + TariTransactionProcessor::new(self.config.clone(), self.template_manager.clone(), fee_table);Please confirm the constructor signature; if it takes ownership of
FeeTable, keep the current code.applications/tari_app_utilities/src/fee_tables.rs (1)
7-24: Mainnet/testnet fee placeholders — gate or codify before release.Values are all
1withper_signature_verification_cost: 10and a TODO. Ensure these are:
- finalized before shipping to MainNet, or
- gated behind config/feature flags, with explicit documentation of units.
Would you like a small config loader to override these via env/cli for non‑prod networks?
crates/engine/src/runtime/impl.rs (1)
210-215: Minor: pass event by reference to avoid needless move/copy.The helper can take
&RuntimeEventand forward that, or deriveCopyon the enum. Either keeps call‑sites simple.Possible tweak:
- fn invoke_modules_on_runtime_event(&self, event: RuntimeEvent) -> Result<(), RuntimeError> { - for module in &self.modules { - module.on_runtime_event(&self.tracker, &event)?; - } + fn invoke_modules_on_runtime_event(&self, event: &RuntimeEvent) -> Result<(), RuntimeError> { + for module in &self.modules { + module.on_runtime_event(&self.tracker, event)?; + } Ok(()) }(Then call with
&RuntimeEvent::SignatureVerified.)crates/template_lib/src/prelude.rs (2)
50-50: Macro re-export is redundant (but harmless).
custom_signature_domainis already#[macro_export]and available at the crate root; re-exporting it via prelude is optional. Keep if you want the ergonomics, otherwise drop to reduce prelude surface.
86-88: Potential prelude breakage:types::*no longer globbed.Switching from
types::*to selective exports can break downstreamuse prelude::*code that relied on othertypes::*items (beyondAmount,amount,crypto). If "no breaking changes" is a goal, either restore a glob export for one cycle or audit dependents.If you want a soft-landing, add this back-compat export just below the current block (outside this range):
pub use crate::types::*;crates/template_lib_types/src/engine_args.rs (1)
14-18: Nameargscould be clearer.
SignatureInvokeArg { args: Vec<Vec<u8>> }carries the encoded payload. Considerpayloadto better communicate intent. Non-blocking.crates/engine/tests/signature.rs (3)
32-35: Strongly type the domain in tests to match the signing hash.You sign with
TEST_DOMAINbut useDerEncodedSignature<NoDomain>. It works over ABI since the domain phantom isn’t serialized, but it defeats compile‑time guarantees. Define a test domain and use it in the signature type.Apply:
const TEST_DOMAIN: &[u8] = b"tari.test.signature domain for tests"; +// Bind the compile-time domain used in signing and verification +custom_signature_domain!(TestDomain, TEST_DOMAIN); -fn sign_it(secret: &RistrettoSecretKey) -> DerEncodedSignature<NoDomain> { +fn sign_it(secret: &RistrettoSecretKey) -> DerEncodedSignature<TestDomain> { sign_it_with(secret, MESSAGE) } -fn sign_it_with(secret: &RistrettoSecretKey, message: &[u8]) -> DerEncodedSignature<NoDomain> { +fn sign_it_with(secret: &RistrettoSecretKey, message: &[u8]) -> DerEncodedSignature<TestDomain> {Note:
custom_signature_domain!is exported by template_lib and available in tests.Also applies to: 37-45
132-132: Avoid unnecessary clone.
assert_reject_reasoncan takereasonby ref/value depending on signature; avoid cloning if not needed.- assert_reject_reason(reason.clone(), "Invalid Ristretto Schnorr signature"); + assert_reject_reason(reason, "Invalid Ristretto Schnorr signature");
97-115: Optional: add assertions inmulti_claim.Currently only checks success. Consider asserting both transfers affected supply/balances as expected.
crates/template_lib/src/models/signature_verifier.rs (2)
12-20: Constructor looks good; consider documenting domain semantics.Add a brief doc on expected domain format and replay-attack rationale to guide template authors.
22-35: Engine call OK; tighten error message.
expect("Failed to decode signature verification result")is fine, but a more specific message helps triage.- resp.decode().expect("Failed to decode signature verification result") + resp.decode().expect("SignatureInvoke decode<bool> failed")Also, you have two impl blocks for the same type; consider merging.
crates/template_lib/src/models/signature.rs (5)
44-56: Doc fix: type name.Example references
DerEncoded, but the type isDerEncodedSignature.-/// let der_signature = DerEncoded::<MyAppDomain>::new(der_bytes); +/// let der_signature = DerEncodedSignature::<MyAppDomain>::new(der_bytes);
56-61: Derive Debug for easier troubleshooting.Add
Debugto aid logging/diagnostics.-#[derive(Clone, serde::Serialize, serde::Deserialize)] +#[derive(Clone, Debug, serde::Serialize, serde::Deserialize)]
63-69: Serialization note.Because
_domainis#[serde(skip)], the domain is not serialized. That’s intentional (callee decides the domain) but can surprise users when crossing ABI boundaries. Consider adding a module‑level note.
71-73: Add a non-consuming accessor for DER bytes.
into_der_bytesconsumesself. Provide a borrowed view to avoid moves/allocs when callers only need a slice.pub fn into_der_bytes(self) -> Vec<u8> { self.der_bytes } + + pub fn as_der_bytes(&self) -> &[u8] { + &self.der_bytes + }
79-83: Panic message is acceptable; consistent with other engine adapters.LGTM. Consider returning
Result<(), Error>in future if you want library-style ergonomics instead of panics.crates/engine/tests/templates/signature/src/lib.rs (3)
54-59: Use Amount-typed literal for the 1000 SIGCOIN capAvoid unit mismatches. Compare against an Amount literal rather than a raw u64.
- if transfer.revealed_input_amount() > 1000_000_000_000u64 { + if transfer.revealed_input_amount() > amount!("1000") { panic!("Cannot claim more than 1000 SIGCOIN at a time"); }
60-65: Order of effects vs. verification — confirm revert semanticsYou remove the key from
allow_listbefore signature verification. If verification fails, is the state guaranteed to fully revert (including the removal) in this engine? If not, the user could lose eligibility due to a bad or malicious call. If revert is guaranteed, consider verifying first for clarity.Also applies to: 71-71
54-58: Inconsistent source of “revealed” input amountYou validate using
transfer.revealed_input_amount()but withdraw usingtransfer.inputs_statement.revealed_amount. Use the same source to prevent drift if implementations diverge.- let input_bucket = self.supply_vault.withdraw(transfer.inputs_statement.revealed_amount); + let input_bucket = self.supply_vault.withdraw(transfer.revealed_input_amount());Also applies to: 73-77
crates/common_types/src/engine_signature.rs (2)
34-49: Preimage structure: add length prefixes or adopt a tagged-hashCurrent preimage is
domain || nonce || public_key || message. To avoid any encoding ambiguities and align with common practice, either:
- Prefix variable-length fields with their lengths; or
- Use a “tagged hash” (hash(domain) twice, then hash concatenation), à la BIP-340.
This hardens domain separation without changing other logic.
Example (length-prefix variant):
- Blake2b::<consts::U64>::new() - .chain_update(domain) + Blake2b::<consts::U64>::new() + .chain_update((domain.len() as u32).to_le_bytes()) + .chain_update(domain) .chain_update(nonce.as_bytes()) .chain_update(public_key.as_bytes()) - .chain_update(message) + .chain_update((message.len() as u32).to_le_bytes()) + .chain_update(message) .finalize()
81-102: Add negative tests for domain and message mismatchesPlease add tests that (a) flip the domain, (b) mutate the message, and (c) tweak the public nonce to ensure verification returns false in each case.
crates/common_types/src/der_signature.rs (2)
10-17: Avoid cloning the signature during encode
encode_ristretto_schnorr_to_vecclones the signature. If possible, encode by reference to avoid copies (small, but easy win).For example, introduce
DerWrapper<&RistrettoSchnorr>and implementDerEncodeValuefor&RistrettoSchnorr, then:- let der_wrapper = DerGenericSignature::new( - SignatureType::RistrettoSchnorrBlake2b, - DerWrapper::new(signature.clone()), - ); + let der_wrapper = DerGenericSignature::new( + SignatureType::RistrettoSchnorrBlake2b, + DerWrapper::new(signature), + );
33-40: Optionally validate that the sequence contains exactly two items
decode_signature_type_from_slicereads the tag andtybut doesn’t enforce trailing structure. Not blocking, but consider validating the sequence structure if this function is used for pre‑dispatch decisions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (39)
Cargo.toml(1 hunks)applications/tari_app_utilities/src/fee_tables.rs(1 hunks)applications/tari_app_utilities/src/lib.rs(1 hunks)applications/tari_indexer/src/dry_run/processor.rs(2 hunks)applications/tari_indexer/src/storage_sqlite/store_factory.rs(1 hunks)applications/tari_indexer/src/substate_manager.rs(1 hunks)applications/tari_validator_node/src/bootstrap.rs(4 hunks)crates/common_types/Cargo.toml(1 hunks)crates/common_types/src/der_signature.rs(1 hunks)crates/common_types/src/engine_signature.rs(1 hunks)crates/common_types/src/lib.rs(2 hunks)crates/engine/Cargo.toml(1 hunks)crates/engine/src/fees/fee_module.rs(2 hunks)crates/engine/src/fees/fee_table.rs(3 hunks)crates/engine/src/runtime/impl.rs(4 hunks)crates/engine/src/runtime/mod.rs(3 hunks)crates/engine/src/runtime/module.rs(1 hunks)crates/engine/src/wasm/process.rs(2 hunks)crates/engine/tests/signature.rs(1 hunks)crates/engine/tests/templates/signature/Cargo.toml(1 hunks)crates/engine/tests/templates/signature/src/lib.rs(1 hunks)crates/engine_types/src/fees.rs(1 hunks)crates/engine_types/src/utxo.rs(1 hunks)crates/template_abi/src/ops.rs(2 hunks)crates/template_lib/src/lib.rs(1 hunks)crates/template_lib/src/models/metadata.rs(1 hunks)crates/template_lib/src/models/mod.rs(2 hunks)crates/template_lib/src/models/signature.rs(1 hunks)crates/template_lib/src/models/signature_verifier.rs(1 hunks)crates/template_lib/src/models/stealth.rs(1 hunks)crates/template_lib/src/prelude.rs(4 hunks)crates/template_lib_types/src/engine_args.rs(1 hunks)crates/template_lib_types/src/lib.rs(1 hunks)crates/template_test_tooling/src/template_test.rs(1 hunks)crates/wallet/sdk/src/apis/stealth_crypto.rs(1 hunks)crates/wallet/sdk/src/models/utxo_update.rs(1 hunks)crates/wallet/storage_sqlite/src/models/stealth_output.rs(1 hunks)crates/wallet/storage_sqlite/src/writer.rs(1 hunks)integration_tests/tests/steps/wallet_daemon.rs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (25)
crates/template_lib/src/models/metadata.rs (2)
bindings/src/types/Metadata.ts (1)
Metadata(6-6)crates/engine_types/src/utxo.rs (5)
from(125-127)from(162-164)from(168-170)new(43-48)new(92-94)
crates/wallet/sdk/src/models/utxo_update.rs (2)
bindings/src/types/RistrettoPublicKeyBytes.ts (1)
RistrettoPublicKeyBytes(6-6)bindings/src/types/UtxoTag.ts (1)
UtxoTag(8-8)
integration_tests/tests/steps/wallet_daemon.rs (4)
bindings/src/types/CommitmentSignatureBytes.ts (1)
CommitmentSignatureBytes(5-5)bindings/src/types/PedersenCommitmentBytes.ts (1)
PedersenCommitmentBytes(6-6)bindings/src/types/Scalar32Bytes.ts (1)
Scalar32Bytes(3-3)bindings/src/types/Amount.ts (1)
Amount(12-12)
applications/tari_indexer/src/substate_manager.rs (2)
bindings/src/types/RistrettoPublicKeyBytes.ts (1)
RistrettoPublicKeyBytes(6-6)bindings/src/types/UtxoTag.ts (1)
UtxoTag(8-8)
crates/template_lib/src/models/stealth.rs (2)
bindings/src/types/StealthTransferStatement.ts (1)
StealthTransferStatement(5-13)bindings/src/types/Amount.ts (1)
Amount(12-12)
applications/tari_indexer/src/storage_sqlite/store_factory.rs (2)
bindings/src/types/RistrettoPublicKeyBytes.ts (1)
RistrettoPublicKeyBytes(6-6)bindings/src/types/UtxoTag.ts (1)
UtxoTag(8-8)
crates/engine_types/src/utxo.rs (2)
crates/template_lib_types/src/entity_id.rs (5)
from_hex(40-42)from_hex(166-168)from_hex(271-281)write_hex_fmt(44-49)write_hex_fmt(170-175)crates/template_lib_types/src/crypto/range_proof.rs (1)
serde_helpers(66-66)
crates/wallet/storage_sqlite/src/models/stealth_output.rs (2)
bindings/src/types/RistrettoPublicKeyBytes.ts (1)
RistrettoPublicKeyBytes(6-6)bindings/src/types/UtxoTag.ts (1)
UtxoTag(8-8)
crates/common_types/src/engine_signature.rs (2)
crates/template_lib/src/models/signature_verifier.rs (1)
verify(23-35)crates/common_types/src/der_signature.rs (4)
new(58-60)new(109-111)decode_ristretto_schnorr_from_slice(20-31)encode_ristretto_schnorr_to_vec(11-17)
crates/wallet/storage_sqlite/src/writer.rs (9)
bindings/src/types/ComponentAddress.ts (1)
ComponentAddress(6-6)bindings/src/types/EncryptedData.ts (1)
EncryptedData(7-7)bindings/src/types/NonFungibleId.ts (1)
NonFungibleId(6-6)bindings/src/types/ResourceAddress.ts (1)
ResourceAddress(6-6)bindings/src/types/VaultId.ts (1)
VaultId(6-6)bindings/src/types/PedersenCommitmentBytes.ts (1)
PedersenCommitmentBytes(6-6)bindings/src/types/RistrettoPublicKeyBytes.ts (1)
RistrettoPublicKeyBytes(6-6)bindings/src/types/UtxoTag.ts (1)
UtxoTag(8-8)bindings/src/types/Amount.ts (1)
Amount(12-12)
applications/tari_app_utilities/src/fee_tables.rs (1)
bindings/src/types/Network.ts (1)
Network(6-6)
crates/template_lib/src/prelude.rs (4)
applications/tari_validator_node_cli/src/command/transaction.rs (1)
amount(697-697)crates/engine_types/src/resource_container.rs (1)
amount(134-141)crates/template_lib/src/models/bucket.rs (1)
amount(176-184)bindings/src/types/Amount.ts (1)
Amount(12-12)
crates/template_lib/src/models/signature_verifier.rs (5)
bindings/src/types/RistrettoPublicKeyBytes.ts (1)
RistrettoPublicKeyBytes(6-6)crates/template_lib/src/models/signature.rs (2)
domain(10-10)domain(26-28)crates/common_types/src/engine_signature.rs (2)
verify(27-27)verify(53-66)crates/template_test_tooling/src/template_test.rs (1)
public_key(394-396)networking/rpc_framework/src/message.rs (1)
message(45-47)
crates/engine/src/runtime/module.rs (1)
crates/engine/src/fees/fee_module.rs (1)
on_runtime_event(71-82)
applications/tari_indexer/src/dry_run/processor.rs (2)
crates/engine/src/state_store/bootstrap.rs (1)
new_memory_store(18-24)applications/tari_app_utilities/src/fee_tables.rs (1)
get_fee_table_by_network(26-35)
applications/tari_validator_node/src/bootstrap.rs (1)
applications/tari_app_utilities/src/fee_tables.rs (1)
get_fee_table_by_network(26-35)
crates/engine/src/fees/fee_module.rs (1)
crates/engine/src/runtime/module.rs (1)
on_runtime_event(19-21)
crates/engine/tests/templates/signature/src/lib.rs (7)
bindings/src/types/RistrettoPublicKeyBytes.ts (1)
RistrettoPublicKeyBytes(6-6)bindings/src/types/Vault.ts (1)
Vault(5-5)crates/template_lib/src/models/signature.rs (1)
new(64-69)crates/template_lib/src/models/stealth.rs (1)
new(49-59)crates/template_lib/src/auth/access_rules.rs (1)
allow_all(137-142)crates/template_lib/src/models/vault.rs (1)
from_bucket(216-221)bindings/src/types/StealthTransferStatement.ts (1)
StealthTransferStatement(5-13)
crates/template_lib/src/models/signature.rs (2)
bindings/src/types/RistrettoPublicKeyBytes.ts (1)
RistrettoPublicKeyBytes(6-6)crates/template_lib/src/models/signature_verifier.rs (1)
with_domain(17-19)
crates/engine/src/runtime/mod.rs (1)
crates/engine/src/runtime/impl.rs (1)
signature_invoke(2630-2658)
crates/template_lib_types/src/engine_args.rs (1)
bindings/src/types/RistrettoPublicKeyBytes.ts (1)
RistrettoPublicKeyBytes(6-6)
crates/wallet/sdk/src/apis/stealth_crypto.rs (5)
bindings/src/types/PedersenCommitmentBytes.ts (1)
PedersenCommitmentBytes(6-6)bindings/src/types/RistrettoPublicKeyBytes.ts (1)
RistrettoPublicKeyBytes(6-6)bindings/src/types/CommitmentSignatureBytes.ts (1)
CommitmentSignatureBytes(5-5)bindings/src/types/UtxoTag.ts (1)
UtxoTag(8-8)bindings/src/types/Amount.ts (1)
Amount(12-12)
crates/engine/tests/signature.rs (7)
crates/template_test_tooling/src/template_test.rs (4)
public_key(394-396)new(83-85)result(432-437)result(465-470)crates/common_types/src/engine_signature.rs (1)
hash_message(34-49)crates/common_types/src/der_signature.rs (3)
encode_ristretto_schnorr_to_vec(11-17)new(58-60)new(109-111)crates/engine/tests/templates/signature/src/lib.rs (1)
new(21-38)crates/engine_types/src/utxo.rs (3)
new(43-48)new(92-94)output(50-52)crates/template_lib/src/models/signature.rs (1)
new(64-69)crates/template_test_tooling/src/support/stealth.rs (1)
generate_transfer_data(103-120)
crates/engine/src/runtime/impl.rs (3)
crates/engine/src/runtime/mod.rs (1)
signature_invoke(195-195)crates/common_types/src/der_signature.rs (1)
decode_signature_type_from_slice(34-40)crates/template_lib/src/args/result.rs (1)
encode(17-20)
crates/common_types/src/der_signature.rs (1)
crates/template_lib/src/models/signature.rs (1)
new(64-69)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: check nightly
- GitHub Check: clippy
- GitHub Check: test
🔇 Additional comments (23)
crates/template_lib/src/models/metadata.rs (1)
84-88: Ergonomic empty-constructor via From<()> looks good; consider a doc blurb and tiny test.This is safe and improves API ergonomics in
Into<Metadata>call sites. To avoid accidental surprises, a short note and a unit test would help.Apply this optional doc comment:
+/// Allows constructing empty `Metadata` from the unit type for ergonomic APIs that accept `Into<Metadata>`. impl From<()> for Metadata { fn from(_: ()) -> Self { Self::new() } }Add a lightweight test to lock behavior in:
#[cfg(test)] mod tests { use super::*; #[test] fn from_unit_produces_empty_metadata() { let m: Metadata = ().into(); assert!(m.into_iter().next().is_none()); } }If any APIs semantically require non-empty metadata, consider validating at those call sites.
applications/tari_indexer/src/substate_manager.rs (1)
45-49: LGTM: import path reorg.Imports moved to
types::crypto/typesare consistent; no behavior change.applications/tari_indexer/src/storage_sqlite/store_factory.rs (1)
27-31: LGTM: import path reorg.Matches the
types::cryptorefactor; no functional impact.applications/tari_app_utilities/src/lib.rs (1)
26-26: LGTM — public fee_tables module exposure verifiedConfirmed applications/tari_app_utilities/src/fee_tables.rs defines
pub const fn get_fee_table_by_network(network: Network) -> &'static FeeTableat line 26; thepub mod fee_tables;export is correct.crates/engine_types/src/fees.rs (1)
72-73: Exhaustively handle FeeSource::SignatureVerification
- Handled: crates/engine/src/fees/fee_module.rs:73-76 adds a FeeSource::SignatureVerification fee charge.
- Action: Verify/update every match over FeeSource and any TypeScript/UI consumers to handle SignatureVerification.
crates/template_abi/src/ops.rs (1)
44-45: ABI op added — dispatch & runtime present; add docs/tests
- Verified: EngineOp::SignatureInvoke is dispatched (crates/engine/src/wasm/process.rs ~213–215), the runtime trait and impl exist (crates/engine/src/runtime/mod.rs and impl.rs ~195, ~2630–2639), types are in crates/template_lib_types/src/engine_args.rs and the call site is in crates/template_lib/src/models/signature_verifier.rs.
- Action: Add unit/integration tests covering SignatureInvoke, update docs/spec to mention the new op, and add a short doc comment on SignatureInvoke describing the expected args layout.
crates/engine_types/src/utxo.rs (1)
14-20: Re-exports confirmed — no action required. Tari_template_lib publicly re-exports tari_template_lib_types astypes(crates/template_lib/src/lib.rs);from_hex,hex::write_hex_fmt,serde_helpersandKeyParseErrorare defined in template_lib_types and are accessible viatari_template_lib::types; engine_types depends ontari_template_lib(crates/engine_types/Cargo.toml) and no dependency cycle was detected.crates/engine/Cargo.toml (1)
13-13: Confirmed: tari_crypto v0.22.0 exposes aserdefeature. Enabling it adds Serialize/Deserialize support for the crate's structs, so the Cargo.toml line is valid.crates/template_lib_types/src/lib.rs (1)
7-7: Module export LGTM
pub mod engine_args;export looks correct and aligns with engine runtime usage.crates/wallet/storage_sqlite/src/models/stealth_output.rs (1)
8-11: Import path update LGTMConsistent with the
types::cryptoconsolidation; no behavioral change.crates/wallet/sdk/src/models/utxo_update.rs (1)
9-9: Import path update LGTMMatches the
types::cryptore-org; no behavior change.crates/common_types/Cargo.toml (1)
24-24: DER features: confirm no_std/wasm consumers before disablingstdrg over Cargo.toml found no
no_stdoccurrences and many workspace crates depend on tari_ootle_common_types — disablingder's defaultstdcould break downstream consumers. Change only if you intentionally need no_std/wasm support.-der = { workspace = true, features = ["std", "derive"] } +der = { workspace = true, default-features = false, features = ["derive", "alloc"] }crates/wallet/sdk/src/apis/stealth_crypto.rs (1)
30-34: Import path refactor looks correct.The move of CommitmentSignatureBytes and UtxoTag to types::crypto aligns with the broader re-org. No functional impact here.
integration_tests/tests/steps/wallet_daemon.rs (1)
11-14: Import updates LGTM.Switching crypto/Amount types to tari_template_lib::types and types::crypto is consistent with the new module layout.
crates/wallet/storage_sqlite/src/writer.rs (1)
45-51: Import consolidation looks good.ComponentAddress to models and crypto types to types::crypto are consistent with the refactor. No behavior change.
crates/template_lib/src/models/mod.rs (1)
38-41: No action required — macros are exported.
crates/template_lib/src/models/signature.rs defines #[macro_export] for macro_rules! custom_signature_domain, so public macros are available to downstream template crates.applications/tari_validator_node/src/bootstrap.rs (1)
40-40: Fee table wiring: drop redundant .clone() when passing &FeeTableIf TariTransactionProcessor::new accepts &FeeTable, pass
fee_table(notfee_table.clone()); verify the constructor signature.File: applications/tari_validator_node/src/bootstrap.rs — lines: 40, 58, 302-309
- let fee_table = get_fee_table_by_network(config.network); + let fee_table = get_fee_table_by_network(config.network); let payload_processor = TariTransactionProcessor::new( TransactionProcessorConfig::new(config.network) .with_template_binary_max_size_bytes(consensus_constants.template_binary_max_size_bytes), template_manager.clone(), - fee_table.clone(), + fee_table, );applications/tari_app_utilities/src/fee_tables.rs (1)
26-35: Network mapping looks exhaustive; keep in sync with bindings.Mapping covers LocalNet/Igor/Esmeralda/StageNet/NextNet/MainNet. Ensure no drift with other
Networkdefs (e.g., TS bindings).crates/engine/src/runtime/mod.rs (1)
195-196: Breaking change: new required RuntimeInterface method — verify all impls implement signature_invokeTrait added at crates/engine/src/runtime/mod.rs:195. Found implementation at crates/engine/src/runtime/impl.rs:2630. ast-grep failed (invalid '-n'), so exhaustive verification didn't complete — re-run a repo-wide search to confirm no other impls are missing signature_invoke.
crates/template_lib/src/prelude.rs (1)
65-76: Good public surface for signature verification.Re-exporting
DerEncodedSignature,RistrettoSchnorrBlake2bSignatureVerifier, andSignatureDomainis the right API shape for template authors.crates/template_lib_types/src/engine_args.rs (2)
20-26: Borrowed args shape is correct for zero-copy serialization.
SignatureVerifyArg<'a>borrows all fields and derivesSerializeonly. This matches theinvoke_args!usage and avoids extra allocations.Please confirm no call sites attempt to deserialize this borrowed form.
28-34: Owned variant correctly mirrors the borrowed struct for engine-side decode.
SignatureVerifyArgOwnedderivingDeserializeis appropriate for the engine boundary.crates/engine/tests/templates/signature/src/lib.rs (1)
22-27: Permissive minting and access rules — confirm test-only scope
mintable(rule!(allow_all))and.with_access_rules(AccessRules::allow_all())are fine for a test template. Please confirm there’s no path for this module to be reused in non-test builds.Also applies to: 36-38
| @@ -0,0 +1,14 @@ | |||
| [workspace] | |||
There was a problem hiding this comment.
🧩 Analysis chain
Remove [workspace] from a workspace member crate.
Declaring a new [workspace] in a nested member will confuse Cargo and can break builds. This test crate should rely on the top-level workspace.
Apply:
-[workspace]🏁 Script executed:
#!/bin/bash
# Fail if any non-root Cargo.toml contains a [workspace] table
rg -n '^\[workspace\]' -g '!target/**' | sed 's/^/Found: /'Length of output: 148
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Finding Cargo.toml files (excluding target/)..."
files=()
while IFS= read -r -d '' f; do files+=("$f"); done < <(find . -type f -name Cargo.toml -not -path '*/target/*' -print0)
if [ ${#files[@]} -eq 0 ]; then
echo "No Cargo.toml files found."
exit 0
fi
echo "Checking non-root Cargo.toml files for a [workspace] table..."
found=0
for f in "${files[@]}"; do
f_rel="${f#./}"
if [ "$f_rel" = "Cargo.toml" ]; then
echo "Skipping root Cargo.toml: $f"
if grep -nP '^\s*\[workspace\]' "$f" >/dev/null 2>&1; then
echo "Root Cargo.toml contains a [workspace] table (expected)."
fi
continue
fi
if grep -nP '^\s*\[workspace\]' "$f" >/dev/null 2>&1; then
found=$((found+1))
echo "Found: $f"
grep -nP '^\s*\[workspace\]' "$f" | sed "s|^|$f:|"
fi
done
if [ $found -eq 0 ]; then
echo "No [workspace] tables found in non-root Cargo.toml files."
else
echo "Total non-root Cargo.toml files with [workspace]: $found"
fi
# Show the specific file referenced in the review, if present
if [ -f "crates/engine/tests/templates/signature/Cargo.toml" ]; then
echo "---- Begin: crates/engine/tests/templates/signature/Cargo.toml (first 120 lines) ----"
sed -n '1,120p' crates/engine/tests/templates/signature/Cargo.toml || true
echo "---- End: crates/engine/tests/templates/signature/Cargo.toml ----"
else
echo "File crates/engine/tests/templates/signature/Cargo.toml not found."
fiLength of output: 104
🏁 Script executed:
#!/bin/bash
set -euo pipefail
target="crates/engine/tests/templates/signature/Cargo.toml"
if [ -f "$target" ]; then
echo "FOUND_FILE: $target"
echo "---- file head (first 40 lines) ----"
sed -n '1,40p' "$target" || true
echo "---- end file head ----"
echo
echo "WORKSPACE_MATCHES (lines matching ^[[:space:]]*\\[workspace\\]):"
grep -nE '^[[:space:]]*\[workspace\]' "$target" || true
exit 0
fi
echo "File not found: $target"
echo "Searching for Cargo.toml files (excluding target/)..."
find . -type f -name Cargo.toml -not -path '*/target/*' -print | sed 's|^\./||' | while IFS= read -r f; do
echo "---- $f (first 20 lines) ----"
sed -n '1,20p' "$f" || true
echo
doneLength of output: 488
Remove [workspace] from workspace member crate
crates/engine/tests/templates/signature/Cargo.toml — line 1 contains a top-level [workspace]; remove it so this member crate inherits the top-level workspace.
Apply:
-[workspace]📝 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.
| [workspace] |
🤖 Prompt for AI Agents
In crates/engine/tests/templates/signature/Cargo.toml around lines 1 to 1, the
file incorrectly declares a top-level [workspace]; remove that [workspace] table
so this member crate does not define its own workspace and instead inherits the
repository-level workspace. Leave any package or dependency sections intact (or
add a [package] section if this is intended to be a crate file), and ensure
there are no other workspace keys in the file.
Test Results (CI)413 tests - 18 413 ✅ - 11 46m 7s ⏱️ - 36m 10s Results for commit 6cb524b. ± Comparison against base commit 1717bd4. This pull request removes 24 and adds 6 tests. Note that renamed tests count towards both. |
6cb524b to
ee3d013
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
crates/engine/src/runtime/impl.rs (1)
2629-2647: Charge only after successful arg decode; current ordering can charge on malformed inputs.RuntimeEvent::SignatureVerified is emitted before args.assert_one_arg()?; decode errors will still trigger the event/fee. Emit the event after successful decode (and before or after verify per policy).
Apply this diff:
fn signature_invoke(&self, action: SignatureAction, args: EngineArgs) -> Result<InvokeResult, RuntimeError> { self.invoke_modules_on_runtime_call("signature_invoke")?; match action { SignatureAction::Verify => { - self.invoke_modules_on_runtime_event(RuntimeEvent::SignatureVerified)?; - let SignatureVerifyArg { public_key, domain, message, payload, } = args.assert_one_arg()?; + + // Charge only once args are well-formed + self.invoke_modules_on_runtime_event(RuntimeEvent::SignatureVerified)?; let is_valid = payload.get_verifier().verify(&domain, &message, &public_key, &payload); Ok(InvokeResult::encode(&is_valid)?) }, } }If the intent is to charge on any attempt (even invalid payload), consider renaming the event to SignatureVerificationAttempted for clarity, or add a separate event for “attempted” vs “successful.”
#!/bin/bash # Verify where SignatureVerified is handled and whether fees are attached to it. rg -n "SignatureVerified" -C3 rg -n "on_runtime_event" -g "crates/engine/src/fees/**" -C3
🧹 Nitpick comments (9)
crates/template_lib_types/src/crypto/signature.rs (4)
8-11: Consider adding documentation for the trait method.The
SignatureDomaintrait would benefit from documentation explaining the purpose and usage of the domain bytes.pub trait SignatureDomain { + /// Returns the domain separation bytes for this signature context. + /// Domain separation helps prevent signature replay attacks across different contexts. fn domain() -> &'static [u8]; }
12-28: Security concern: Document risks ofNoSignatureDomainmore prominently.While the warning comment is present, the security implications of using an empty domain should be more prominent. Consider making this type harder to use accidentally.
Consider either:
- Renaming to
UnsafeNoSignatureDomainto make the risk more obvious- Adding a
#[deprecated]attribute with a warning message- Making it available only behind a feature flag like
unsafe-empty-domain/// A signature domain that is the empty byte string. /// -/// # Warning +/// # ⚠️ Security Warning /// This is not recommended use as it could lead to signature replay attacks across different contexts. +/// Using an empty domain allows signatures to be replayed across different applications or contexts, +/// potentially leading to unauthorized actions. /// Instead, define a custom domain for your application using the `custom_signature_domain!` macro.
56-67: Consider deriving additional standard traits.The
Signature<D>struct could benefit from derivingDebugfor better debugging experience.-#[derive(Clone, serde::Serialize, serde::Deserialize)] +#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] pub struct Signature<D> { payload: SignaturePayload, #[serde(skip)] _domain: PhantomData<D>, }
125-132: Consider adding a domain accessor method.While
_domainis intentionally private, it might be useful to provide a method to retrieve the domain bytes for debugging or verification purposes.impl<D: SignatureDomain> Signature<D> { pub fn new<T: Into<SignaturePayload>>(payload: T) -> Self { Self { payload: payload.into(), _domain: PhantomData, } } + + /// Returns the domain separation bytes for this signature. + pub fn domain() -> &'static [u8] { + D::domain() + } }crates/engine/tests/templates/signature/src/lib.rs (2)
42-45: Boolean verify helper is fine.If you don’t need a custom message, you could use spend_signature.assert_valid(public_key, SOME_MESSAGE) to fail fast, but returning bool here is intentional.
70-72: Minor: unnecessary conversion.public_key is already a PublicKey; .into() is redundant.
- if !spend_signature.verify(&public_key.into(), SOME_MESSAGE) { + if !spend_signature.verify(&public_key, SOME_MESSAGE) {crates/engine/tests/signature.rs (1)
35-36: Centralize signature domain (avoid drift)TEST_DOMAIN duplicates the domain used by the template's custom_signature_domain! — export the domain as a single const from crates/engine/tests/templates/signature/src/lib.rs and use it in crates/engine/tests/signature.rs, or add a clear sync comment linking the two locations.
Locations: crates/engine/tests/signature.rs:35, crates/engine/tests/templates/signature/src/lib.rs:8.crates/common_types/src/engine_signature.rs (2)
66-67: Consider adding domain separation validation.While the current implementation accepts any domain bytes, consider whether domain validation or length restrictions should be enforced to prevent potential misuse or domain collision attacks.
You might want to add domain validation:
// Consider adding validation like: if domain.is_empty() { return false; // Prevent empty domain usage } if domain.len() > MAX_DOMAIN_LENGTH { return false; // Prevent excessively long domains }
89-111: Consider adding negative test cases.The current test only covers the valid signature path. Consider adding tests for invalid signatures, wrong domains, corrupted payloads, and mismatched keys to ensure robust error handling.
Add comprehensive negative test cases:
#[test] fn it_rejects_invalid_signature() { // Test with wrong domain, corrupted signature, mismatched keys, etc. } #[test] fn it_rejects_corrupted_signature_payload() { // Test with malformed signature bytes } #[test] fn it_rejects_wrong_public_key() { // Test with different public key }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (37)
applications/tari_app_utilities/src/fee_tables.rs(1 hunks)applications/tari_app_utilities/src/lib.rs(1 hunks)applications/tari_indexer/src/dry_run/processor.rs(2 hunks)applications/tari_indexer/src/storage_sqlite/store_factory.rs(1 hunks)applications/tari_indexer/src/substate_manager.rs(1 hunks)applications/tari_validator_node/src/bootstrap.rs(4 hunks)crates/common_types/src/engine_signature.rs(1 hunks)crates/common_types/src/lib.rs(2 hunks)crates/engine/Cargo.toml(1 hunks)crates/engine/src/fees/fee_module.rs(2 hunks)crates/engine/src/fees/fee_table.rs(3 hunks)crates/engine/src/runtime/impl.rs(4 hunks)crates/engine/src/runtime/mod.rs(3 hunks)crates/engine/src/runtime/module.rs(1 hunks)crates/engine/src/wasm/process.rs(2 hunks)crates/engine/tests/signature.rs(1 hunks)crates/engine/tests/templates/signature/Cargo.toml(1 hunks)crates/engine/tests/templates/signature/src/lib.rs(1 hunks)crates/engine_types/src/fees.rs(1 hunks)crates/engine_types/src/utxo.rs(1 hunks)crates/template_abi/src/ops.rs(2 hunks)crates/template_lib/src/lib.rs(1 hunks)crates/template_lib/src/models/metadata.rs(1 hunks)crates/template_lib/src/models/mod.rs(2 hunks)crates/template_lib/src/models/signature_verifier.rs(1 hunks)crates/template_lib/src/models/stealth.rs(1 hunks)crates/template_lib/src/prelude.rs(2 hunks)crates/template_lib_types/src/crypto/mod.rs(1 hunks)crates/template_lib_types/src/crypto/signature.rs(1 hunks)crates/template_lib_types/src/engine_args.rs(1 hunks)crates/template_lib_types/src/lib.rs(1 hunks)crates/template_test_tooling/src/template_test.rs(1 hunks)crates/wallet/sdk/src/apis/stealth_crypto.rs(1 hunks)crates/wallet/sdk/src/models/utxo_update.rs(1 hunks)crates/wallet/storage_sqlite/src/models/stealth_output.rs(1 hunks)crates/wallet/storage_sqlite/src/writer.rs(1 hunks)integration_tests/tests/steps/wallet_daemon.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (26)
- applications/tari_app_utilities/src/lib.rs
- crates/wallet/sdk/src/models/utxo_update.rs
- crates/engine/src/wasm/process.rs
- crates/template_lib/src/models/metadata.rs
- crates/engine/Cargo.toml
- crates/template_lib/src/lib.rs
- crates/engine_types/src/utxo.rs
- applications/tari_indexer/src/storage_sqlite/store_factory.rs
- crates/template_lib/src/models/mod.rs
- crates/template_abi/src/ops.rs
- crates/engine/src/fees/fee_table.rs
- applications/tari_app_utilities/src/fee_tables.rs
- crates/wallet/storage_sqlite/src/models/stealth_output.rs
- crates/engine/tests/templates/signature/Cargo.toml
- integration_tests/tests/steps/wallet_daemon.rs
- crates/engine/src/runtime/module.rs
- crates/template_test_tooling/src/template_test.rs
- crates/engine_types/src/fees.rs
- crates/wallet/sdk/src/apis/stealth_crypto.rs
- applications/tari_indexer/src/dry_run/processor.rs
- crates/engine/src/runtime/mod.rs
- crates/template_lib/src/prelude.rs
- crates/engine/src/fees/fee_module.rs
- crates/template_lib/src/models/stealth.rs
- applications/tari_validator_node/src/bootstrap.rs
- applications/tari_indexer/src/substate_manager.rs
🧰 Additional context used
🧬 Code graph analysis (7)
crates/wallet/storage_sqlite/src/writer.rs (9)
bindings/src/types/ComponentAddress.ts (1)
ComponentAddress(6-6)bindings/src/types/EncryptedData.ts (1)
EncryptedData(7-7)bindings/src/types/NonFungibleId.ts (1)
NonFungibleId(6-6)bindings/src/types/ResourceAddress.ts (1)
ResourceAddress(6-6)bindings/src/types/VaultId.ts (1)
VaultId(6-6)bindings/src/types/PedersenCommitmentBytes.ts (1)
PedersenCommitmentBytes(6-6)bindings/src/types/RistrettoPublicKeyBytes.ts (1)
RistrettoPublicKeyBytes(6-6)bindings/src/types/UtxoTag.ts (1)
UtxoTag(8-8)bindings/src/types/Amount.ts (1)
Amount(12-12)
crates/common_types/src/engine_signature.rs (2)
crates/template_lib/src/models/signature_verifier.rs (3)
verify(13-13)verify(23-25)verify(38-50)crates/template_lib_types/src/crypto/signature.rs (2)
domain(9-9)domain(25-27)
crates/engine/tests/signature.rs (5)
crates/template_test_tooling/src/template_test.rs (4)
public_key(394-396)new(83-85)result(432-437)result(465-470)crates/common_types/src/engine_signature.rs (1)
compute_challenge(33-48)crates/engine/tests/templates/signature/src/lib.rs (1)
new(23-40)crates/template_lib_types/src/crypto/signature.rs (4)
new(126-131)from(64-66)from(82-84)from(120-122)crates/template_test_tooling/src/support/stealth.rs (1)
generate_transfer_data(103-120)
crates/template_lib_types/src/crypto/signature.rs (2)
bindings/src/types/RistrettoPublicKeyBytes.ts (1)
RistrettoPublicKeyBytes(6-6)bindings/src/types/SchnorrSignatureBytes.ts (1)
SchnorrSignatureBytes(5-5)
crates/engine/tests/templates/signature/src/lib.rs (5)
bindings/src/types/Vault.ts (1)
Vault(5-5)crates/template_lib/src/models/stealth.rs (1)
new(49-59)crates/template_lib_types/src/crypto/signature.rs (1)
new(126-131)crates/template_lib/src/models/vault.rs (1)
from_bucket(216-221)bindings/src/types/StealthTransferStatement.ts (1)
StealthTransferStatement(5-13)
crates/template_lib/src/models/signature_verifier.rs (2)
crates/common_types/src/engine_signature.rs (2)
verify(26-26)verify(52-75)crates/template_lib_types/src/crypto/signature.rs (3)
domain(9-9)domain(25-27)payload(135-137)
crates/engine/src/runtime/impl.rs (4)
crates/engine/src/runtime/mod.rs (1)
signature_invoke(195-195)crates/template_lib_types/src/crypto/signature.rs (3)
payload(135-137)domain(9-9)domain(25-27)crates/template_test_tooling/src/template_test.rs (1)
public_key(394-396)crates/transaction/src/v1/signature.rs (2)
public_key(63-65)public_key(122-124)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: clippy
- GitHub Check: check nightly
- GitHub Check: check stable
- GitHub Check: file licenses
- GitHub Check: test
- GitHub Check: machete
- GitHub Check: fmt
🔇 Additional comments (19)
crates/template_lib_types/src/crypto/mod.rs (1)
15-17: LGTM! Module organization follows Rust conventions.The addition of the signature module with
#[macro_use]attribute and public re-export is appropriate for making thecustom_signature_domain!macro available throughout the crate.crates/wallet/storage_sqlite/src/writer.rs (1)
44-51: LGTM! Import restructuring aligns with the new module organization.The migration of types to their appropriate modules (
modelsfor domain entities,types::cryptofor cryptographic primitives) improves code organization and follows the principle of separation of concerns.crates/template_lib_types/src/crypto/signature.rs (2)
30-41: Well-designed macro for domain creation.The
custom_signature_domain!macro provides a clean and ergonomic way to define custom signature domains, encouraging secure practices.
102-108: Security consideration: Empty bytes forZeropublic key.Returning an empty slice for
PublicKey::Zeroinas_bytes()could be problematic if this is used in cryptographic operations. Consider whether this should panic or return an error instead.Can you verify how
PublicKey::Zerois intended to be used? If it represents an invalid or uninitialized key, returning empty bytes might lead to subtle security issues if not properly handled by callers.crates/common_types/src/lib.rs (1)
11-11: LGTM! Clean module addition following existing patterns.The addition of the private
engine_signaturemodule with public re-export of its contents follows the established pattern in this file.Also applies to: 39-39
crates/template_lib_types/src/lib.rs (1)
4-9: LGTM! Proper module declarations for signature support.The addition of
#[macro_use]for the crypto module and the newengine_argsmodule properly exposes the signature verification functionality. The macro_use attribute is necessary for making thecustom_signature_domain!macro available throughout the crate.crates/engine/src/runtime/impl.rs (2)
61-63: Imports and type wiring look correct.The added GetVerifier import and new engine_args types are consistent with usage below.
Also applies to: 127-133, 136-136
209-214: Runtime event dispatch helper LGTM.Straightforward loop over modules; no issues spotted.
crates/engine/tests/signature.rs (4)
35-51: Good: deterministic signing flow matches engine verifier.Uses compute_challenge with the same domain, nonce, pk ordering as verifier; nonce from OsRng is correct for Schnorr.
68-97: End-to-end valid-claim test looks solid.Covers UTXO creation and vault balance delta; assertions are clear.
120-137: Negative test is precise and helpful.Checks reject reason string; good coverage of the error path.
139-171: API checks are comprehensive.Covers bad message, good sig, wrong key in one transaction; decoding assertions are tight.
crates/template_lib_types/src/engine_args.rs (1)
14-34: Args structs and (de)serialization LGTM.Borrowed/owned split is appropriate for ABI; field names align with engine usage.
crates/engine/tests/templates/signature/src/lib.rs (2)
53-58: Bounds checks read well.Limit matches 1_000 SIGCOIN given 9 divisibility.
74-83: Transfer flow is correct.Withdraw revealed amount, perform stealth transfer with optional input bucket, re-deposit any revealed outputs. Good anti-replay via allow_list mutation.
crates/template_lib/src/models/signature_verifier.rs (2)
12-20: Trait shape LGTM.Simple API with assert helper; suitable for templates.
37-51: Engine round-trip decode panic is acceptable here.Decoding failure indicates an engine/runtime bug; panicking is fine for templates. No changes needed.
crates/common_types/src/engine_signature.rs (2)
52-76: Robust error handling in signature verification.The verification method properly handles potential conversion failures and returns
falserather than panicking. The pattern matching and error handling for signature payload extraction is appropriate.
89-111: Comprehensive test coverage for the happy path.The test properly demonstrates the end-to-end signature verification flow, including key generation, message hashing with domain separation, signature creation, and verification. The test follows cryptographic best practices by using proper randomness sources.
* development: feat(template_lib): adds engine schnorr signature verification (tari-project#1574) feat(wallet)!: add bech32 address with view-only key (tari-project#1573) feat(walletui): wallet ux improvements (tari-project#1572) fix(wallet)!: private derived tag and optimised* sync protocol (tari-project#1571) feat(walletui): send flow ux improvements (tari-project#1570) doc: update openrpc.json get_connections method (tari-project#1567) chore(deps): bump actions/setup-node from 4 to 5 (tari-project#1565)
2 participants
Description
Allows users to verify Schnorr signatures programmatically in templates.
Added a runtime events hook to allow for special fee charges
Charge a flat fee whenever a signature is verified.
Fee table definition per network
DRYed up fee table definitions
Simple domain separation (template_lib should be minimal to avoid large WASM sizes)
Motivation and Context
Expect several use cases, for instance, signed voting, and authorisation without badges by sending the user an out-of-band signature.
Notes:
H(R||P||'ABC')Ref #1519
How Has This Been Tested?
Signature unit tests
What process can a PR reviewer use to test or verify this change?
Create a template that uses the new signature API
Breaking Changes
Summary by CodeRabbit
New Features
Refactor
Tests