From 19fed267e31261dbc3446d970aae2feb0e753bb0 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Thu, 6 Apr 2023 17:21:58 +0000 Subject: [PATCH 1/2] Use `cxx` bridge for all Orchard bundle inspection and validation zcash/zcash#5987 added a bridge to `orchard::Bundle` for `getrawtransaction`. This commit expands it to also cover the consensus rules, by migrating over missing functionality from the hand-written FFI methods, and exposing the Orchard `BatchValidator` type directly (as with Sapling) instead of via the C++ `AuthValidator` intermediary. Part of zcash/zcash#6397. --- src/gtest/test_checktransaction.cpp | 8 +- src/main.cpp | 14 +- src/main.h | 2 +- src/primitives/orchard.h | 67 +++--- src/rpc/rawtransaction.cpp | 2 +- src/rust/include/rust/orchard.h | 164 ------------- src/rust/src/bridge.rs | 49 +++- src/rust/src/orchard_bundle.rs | 139 +++++++++++- src/rust/src/orchard_ffi.rs | 341 ++++++---------------------- src/test/transaction_tests.cpp | 2 +- src/wallet/orchard.h | 8 +- 11 files changed, 292 insertions(+), 504 deletions(-) diff --git a/src/gtest/test_checktransaction.cpp b/src/gtest/test_checktransaction.cpp index bcc5e8153..e6eeb070d 100644 --- a/src/gtest/test_checktransaction.cpp +++ b/src/gtest/test_checktransaction.cpp @@ -537,7 +537,7 @@ TEST(ContextualCheckShieldedInputsTest, BadTxnsInvalidJoinsplitSignature) { SelectParams(CBaseChainParams::REGTEST); auto consensus = Params().GetConsensus(); std::optional> saplingAuth = std::nullopt; - auto orchardAuth = orchard::AuthValidator::Disabled(); + std::optional> orchardAuth = std::nullopt; CMutableTransaction mtx = GetValidTransaction(); mtx.joinSplitSig.bytes[0] += 1; @@ -569,7 +569,7 @@ TEST(ContextualCheckShieldedInputsTest, JoinsplitSignatureDetectsOldBranchId) { SelectParams(CBaseChainParams::REGTEST); auto consensus = Params().GetConsensus(); std::optional> saplingAuth = std::nullopt; - auto orchardAuth = orchard::AuthValidator::Disabled(); + std::optional> orchardAuth = std::nullopt; auto saplingBranchId = NetworkUpgradeInfo[Consensus::UPGRADE_SAPLING].nBranchId; auto blossomBranchId = NetworkUpgradeInfo[Consensus::UPGRADE_BLOSSOM].nBranchId; @@ -619,7 +619,7 @@ TEST(ContextualCheckShieldedInputsTest, NonCanonicalEd25519Signature) { SelectParams(CBaseChainParams::REGTEST); auto consensus = Params().GetConsensus(); std::optional> saplingAuth = std::nullopt; - auto orchardAuth = orchard::AuthValidator::Disabled(); + std::optional> orchardAuth = std::nullopt; AssumeShieldedInputsExistAndAreSpendable baseView; CCoinsViewCache view(&baseView); @@ -1325,7 +1325,7 @@ TEST(ChecktransactionTests, HeartwoodEnforcesSaplingRulesOnShieldedCoinbase) { EXPECT_TRUE(ContextualCheckTransaction(tx, state, chainparams, 10, 57)); std::optional> saplingAuth = sapling::init_batch_validator(false); - auto orchardAuth = orchard::AuthValidator::Disabled(); + std::optional> orchardAuth = std::nullopt; auto heartwoodBranchId = NetworkUpgradeInfo[Consensus::UPGRADE_HEARTWOOD].nBranchId; // Coinbase transaction does not pass shielded input checks, as bindingSig diff --git a/src/main.cpp b/src/main.cpp index 4d21bea31..b02b15778 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1286,7 +1286,7 @@ bool ContextualCheckShieldedInputs( CValidationState &state, const CCoinsViewCache &view, std::optional>& saplingAuth, - std::optional& orchardAuth, + std::optional>& orchardAuth, const Consensus::Params& consensus, uint32_t consensusBranchId, bool nu5Active, @@ -1411,7 +1411,7 @@ bool ContextualCheckShieldedInputs( // Queue Orchard bundle to be batch-validated. if (orchardAuth.has_value()) { - tx.GetOrchardBundle().QueueAuthValidation(orchardAuth.value(), dataToBeSigned); + tx.GetOrchardBundle().QueueAuthValidation(*orchardAuth.value(), dataToBeSigned); } return true; @@ -2049,7 +2049,7 @@ bool AcceptToMemoryPool( // This will be a single-transaction batch, which is still more efficient as every // Orchard bundle contains at least two signatures. - std::optional orchardAuth = orchard::AuthValidator::Batch(true); + std::optional> orchardAuth = orchard::init_batch_validator(true); // Check shielded input signatures. if (!ContextualCheckShieldedInputs( @@ -2072,7 +2072,7 @@ bool AcceptToMemoryPool( if (!saplingAuth.value()->validate()) { return state.DoS(100, false, REJECT_INVALID, "bad-sapling-bundle-authorization"); } - if (!orchardAuth.value().Validate()) { + if (!orchardAuth.value()->validate()) { return state.DoS(100, false, REJECT_INVALID, "bad-orchard-bundle-authorization"); } @@ -3155,8 +3155,8 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin // Disable Sapling and Orchard batch validation if possible. std::optional> saplingAuth = fExpensiveChecks ? std::optional(sapling::init_batch_validator(fCacheResults)) : std::nullopt; - std::optional orchardAuth = fExpensiveChecks ? - orchard::AuthValidator::Batch(fCacheResults) : orchard::AuthValidator::Disabled(); + std::optional> orchardAuth = fExpensiveChecks ? + std::optional(orchard::init_batch_validator(fCacheResults)) : std::nullopt; // If in initial block download, and this block is an ancestor of a checkpoint, // and -ibdskiptxverification is set, disable all transaction checks. @@ -3665,7 +3665,7 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin } // Ensure Orchard signatures are valid (if we are checking them) - if (orchardAuth.has_value() && !orchardAuth.value().Validate()) { + if (orchardAuth.has_value() && !orchardAuth.value()->validate()) { return state.DoS(100, error("ConnectBlock(): an Orchard bundle within the block is invalid"), REJECT_INVALID, "bad-orchard-bundle-authorization"); diff --git a/src/main.h b/src/main.h index 270fba025..df0cc243c 100644 --- a/src/main.h +++ b/src/main.h @@ -414,7 +414,7 @@ bool ContextualCheckShieldedInputs( CValidationState &state, const CCoinsViewCache &view, std::optional>& saplingAuth, - std::optional& orchardAuth, + std::optional>& orchardAuth, const Consensus::Params& consensus, uint32_t consensusBranchId, bool nu5Active, diff --git a/src/primitives/orchard.h b/src/primitives/orchard.h index 28b453b1d..20b6d99a5 100644 --- a/src/primitives/orchard.h +++ b/src/primitives/orchard.h @@ -6,11 +6,11 @@ #define ZCASH_PRIMITIVES_ORCHARD_H #include "streams.h" +#include "streams_rust.h" #include #include -#include #include #include "zcash/address/orchard.hpp" @@ -24,22 +24,22 @@ namespace orchard { class UnauthorizedBundle; } class OrchardBundle { private: - /// An optional Orchard bundle (with `nullptr` corresponding to `None`). + /// An optional Orchard bundle. /// Memory is allocated by Rust. - std::unique_ptr inner; + rust::Box inner; - OrchardBundle(OrchardBundlePtr* bundle) : inner(bundle, orchard_bundle_free) {} + OrchardBundle(OrchardBundlePtr* bundle) : inner(orchard_bundle::from_raw_box(bundle)) {} friend class OrchardMerkleFrontier; friend class OrchardWallet; friend class orchard::UnauthorizedBundle; public: - OrchardBundle() : inner(nullptr, orchard_bundle_free) {} + OrchardBundle() : inner(orchard_bundle::none()) {} OrchardBundle(OrchardBundle&& bundle) : inner(std::move(bundle.inner)) {} OrchardBundle(const OrchardBundle& bundle) : - inner(orchard_bundle_clone(bundle.inner.get()), orchard_bundle_free) {} + inner(bundle.inner->box_clone()) {} OrchardBundle& operator=(OrchardBundle&& bundle) { @@ -52,87 +52,88 @@ public: OrchardBundle& operator=(const OrchardBundle& bundle) { if (this != &bundle) { - inner.reset(orchard_bundle_clone(bundle.inner.get())); + inner = bundle.inner->box_clone(); } return *this; } - rust::Box GetDetails() const { - return orchard_bundle::from_tx_bundle(reinterpret_cast(inner.get())); + const rust::Box& GetDetails() const { + return inner; } size_t RecursiveDynamicUsage() const { - return orchard_bundle_recursive_dynamic_usage(inner.get()); + return inner->recursive_dynamic_usage(); } template void Serialize(Stream& s) const { - RustStream rs(s); - if (!orchard_bundle_serialize(inner.get(), &rs, RustStream::write_callback)) { - throw std::ios_base::failure("Failed to serialize v5 Orchard bundle"); + try { + inner->serialize(*ToRustStream(s)); + } catch (const std::exception& e) { + throw std::ios_base::failure(e.what()); } } template void Unserialize(Stream& s) { - RustStream rs(s); - OrchardBundlePtr* bundle; - if (!orchard_bundle_parse(&rs, RustStream::read_callback, &bundle)) { - throw std::ios_base::failure("Failed to parse v5 Orchard bundle"); + try { + inner = orchard_bundle::parse(*ToRustStream(s)); + } catch (const std::exception& e) { + throw std::ios_base::failure(e.what()); } - inner.reset(bundle); } /// Returns true if this contains an Orchard bundle, or false if there is no /// Orchard component. - bool IsPresent() const { return (bool)inner; } + bool IsPresent() const { return inner->is_present(); } /// Returns the net value entering or exiting the Orchard pool as a result of this /// bundle. CAmount GetValueBalance() const { - return orchard_bundle_value_balance(inner.get()); + return inner->value_balance_zat(); } /// Queues this bundle's authorization for validation. /// /// `sighash` must be for the transaction this bundle is within. void QueueAuthValidation( - orchard::AuthValidator& batch, const uint256& sighash) const + orchard::BatchValidator& batch, const uint256& sighash) const { - batch.Queue(inner.get(), sighash.begin()); + batch.add_bundle(inner->box_clone(), sighash.GetRawBytes()); } const size_t GetNumActions() const { - return orchard_bundle_actions_len(inner.get()); + return inner->num_actions(); } const std::vector GetNullifiers() const { - size_t actions_len = orchard_bundle_actions_len(inner.get()); - std::vector result(actions_len); - auto nullifiers_ok = orchard_bundle_nullifiers(inner.get(), result.data(), actions_len); - assert(nullifiers_ok); + const auto actions = inner->actions(); + std::vector result; + result.reserve(actions.size()); + for (const auto& action : actions) { + result.push_back(uint256::FromRawBytes(action.nullifier())); + } return result; } const std::optional GetAnchor() const { - uint256 result; - if (orchard_bundle_anchor(inner.get(), result.begin())) { - return result; + if (IsPresent()) { + return uint256::FromRawBytes(inner->anchor()); } else { return std::nullopt; } } bool OutputsEnabled() const { - return orchard_bundle_outputs_enabled(inner.get()); + return inner->enable_outputs(); } bool SpendsEnabled() const { - return orchard_bundle_spends_enabled(inner.get()); + return inner->enable_spends(); } bool CoinbaseOutputsAreValid() const { - return orchard_bundle_coinbase_outputs_are_valid(inner.get()); + return inner->coinbase_outputs_are_valid(); } }; diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 1645d86d0..308ed175b 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -181,7 +181,7 @@ UniValue TxActionsToJSON(const rust::Vec& actions) // See https://zips.z.cash/zip-0225 UniValue TxOrchardBundleToJSON(const CTransaction& tx, UniValue& entry) { - auto bundle = tx.GetOrchardBundle().GetDetails(); + const auto& bundle = tx.GetOrchardBundle().GetDetails(); UniValue obj(UniValue::VOBJ); auto actions = bundle->actions(); diff --git a/src/rust/include/rust/orchard.h b/src/rust/include/rust/orchard.h index b397c2980..2e6a6995a 100644 --- a/src/rust/include/rust/orchard.h +++ b/src/rust/include/rust/orchard.h @@ -17,172 +17,8 @@ extern "C" { struct OrchardBundlePtr; typedef struct OrchardBundlePtr OrchardBundlePtr; -struct OrchardBatchValidatorPtr; -typedef struct OrchardBatchValidatorPtr OrchardBatchValidatorPtr; - -/// Clones the given Orchard bundle. -/// -/// Both bundles need to be separately freed when they go out of scope. -OrchardBundlePtr* orchard_bundle_clone(const OrchardBundlePtr* bundle); - -/// Frees an Orchard bundle returned from `orchard_parse_bundle`. -void orchard_bundle_free(OrchardBundlePtr* bundle); - -/// Returns the amount of dynamically-allocated memory used by this bundle. -size_t orchard_bundle_recursive_dynamic_usage(const OrchardBundlePtr* bundle); - -/// Parses an authorized Orchard bundle from the given stream. -/// -/// - If no error occurs, `bundle_ret` will point to a Rust-allocated Orchard bundle. -/// - If an error occurs, `bundle_ret` will be unaltered. -bool orchard_bundle_parse( - void* stream, - read_callback_t read_cb, - OrchardBundlePtr** bundle_ret); - -/// Serializes an authorized Orchard bundle to the given stream -/// -/// If `bundle == nullptr`, this serializes `nActionsOrchard = 0`. -bool orchard_bundle_serialize( - const OrchardBundlePtr* bundle, - void* stream, - write_callback_t write_cb); - -/// Returns the value balance for this Orchard bundle. -/// -/// A transaction with no Orchard component has a value balance of zero. -int64_t orchard_bundle_value_balance(const OrchardBundlePtr* bundle); - -/// Returns the number of actions associated with the bundle. -size_t orchard_bundle_actions_len(const OrchardBundlePtr* bundle); - -/// Returns the nullifiers for the bundle by copying them into the provided -/// vector of 32-byte arrays `nullifiers_ret`, which should be sized to the -/// number of anchors. -/// -/// Returns `false` if the number of nullifiers specified varies from the -/// number of the actions in the bundle, `true` otherwise. -bool orchard_bundle_nullifiers( - const OrchardBundlePtr* bundle, - void* nullifiers_ret, - size_t nullifiers_len - ); - -/// Returns the anchor for the bundle by copying them into -/// the provided value. -/// -/// Returns `false` if the bundle was absent, `true` otherwise. -bool orchard_bundle_anchor( - const OrchardBundlePtr* bundle, - unsigned char* anchor_ret); - -/// Initializes a new Orchard batch validator. -/// -/// Please free this with `orchard_batch_validation_free` when you are done with -/// it. -OrchardBatchValidatorPtr* orchard_batch_validation_init(bool cache_store); - -/// Frees a batch validator returned from `orchard_batch_validation_init`. -void orchard_batch_validation_free(OrchardBatchValidatorPtr* batch); - -/// Adds an Orchard bundle to this batch. -void orchard_batch_add_bundle( - OrchardBatchValidatorPtr* batch, - const OrchardBundlePtr* bundle, - const unsigned char* sighash); - -/// Validates this batch. -/// -/// Returns false if any item in the batch is invalid. -/// -/// ## Consensus rules -/// -/// [§4.6](https://zips.z.cash/protocol/protocol.pdf#actiondesc): -/// - Canonical element encodings are enforced by `orchard_bundle_parse`. -/// - SpendAuthSig^Orchard validity is enforced here. -/// - Proof validity is enforced here. -/// -/// [§7.1](https://zips.z.cash/protocol/protocol.pdf#txnencodingandconsensus): -/// - `bindingSigOrchard` validity is enforced here. -bool orchard_batch_validate(const OrchardBatchValidatorPtr* batch); - -/// Returns whether the Orchard bundle is present and outputs -/// are enabled. -bool orchard_bundle_outputs_enabled(const OrchardBundlePtr* bundle); - -/// Returns whether the Orchard bundle is present and spends -/// are enabled. -bool orchard_bundle_spends_enabled(const OrchardBundlePtr* bundle); - -/// Returns whether all actions contained in the Orchard bundle -/// can be decrypted with the all-zeros OVK. Returns `true` -/// if no Orchard actions are present. -bool orchard_bundle_coinbase_outputs_are_valid(const OrchardBundlePtr* bundle); - #ifdef __cplusplus } #endif -#ifdef __cplusplus -namespace orchard -{ -/** - * A validator for the Orchard authorization component of a transaction. - */ -class AuthValidator -{ -private: - /// An optional batch validator (with `nullptr` corresponding to `None`). - /// Memory is allocated by Rust. - std::unique_ptr inner; - - AuthValidator() : inner(nullptr, orchard_batch_validation_free) {} - -public: - // AuthValidator should never be copied - AuthValidator(const AuthValidator&) = delete; - AuthValidator& operator=(const AuthValidator&) = delete; - AuthValidator(AuthValidator&& bundle) : inner(std::move(bundle.inner)) {} - AuthValidator& operator=(AuthValidator&& bundle) - { - if (this != &bundle) { - inner = std::move(bundle.inner); - } - return *this; - } - - /// Creates a validation context that batch-validates Orchard proofs and - /// signatures. - static AuthValidator Batch(bool cacheResult) { - auto batch = AuthValidator(); - batch.inner.reset(orchard_batch_validation_init(cacheResult)); - return batch; - } - - /// Creates a validation context that performs no validation. This can be - /// used when avoiding duplicate effort such as during reindexing. - static std::optional Disabled() { - return std::nullopt; - } - - /// Queues an Orchard bundle for validation. - void Queue(const OrchardBundlePtr* bundle, const unsigned char* sighash) { - orchard_batch_add_bundle(inner.get(), bundle, sighash); - } - - /// Validates the queued Orchard authorizations, returning `true` if all - /// proofs and signatures were valid, and `false` otherwise. - /// - /// Throws `std::logic_error` if called more than once. - bool Validate() { - if (!inner) { - throw std::logic_error("orchard::AuthValidator has already been used"); - } - - return orchard_batch_validate(inner.release()); - } -}; -} // namespace orchard -#endif - #endif // ZCASH_RUST_INCLUDE_RUST_ORCHARD_H diff --git a/src/rust/src/bridge.rs b/src/rust/src/bridge.rs index 88afbeeb7..37d82e072 100644 --- a/src/rust/src/bridge.rs +++ b/src/rust/src/bridge.rs @@ -12,12 +12,15 @@ use crate::{ note_encryption::{ try_sapling_note_decryption, try_sapling_output_recovery, DecryptedSaplingOutput, }, - orchard_bundle::{from_tx_bundle, Action, Bundle, OrchardBundle}, + orchard_bundle::{ + none_orchard_bundle, orchard_bundle_from_raw_box, parse_orchard_bundle, Action, Bundle, + }, + orchard_ffi::{orchard_batch_validation_init, BatchValidator as OrchardBatchValidator}, params::{network, Network}, sapling::{ - finish_bundle_assembly, init_batch_validator, init_prover, init_verifier, - new_bundle_assembler, BatchValidator, Bundle as SaplingBundle, - BundleAssembler as SaplingBundleAssembler, Prover, Verifier, + finish_bundle_assembly, init_batch_validator as init_sapling_batch_validator, init_prover, + init_verifier, new_bundle_assembler, BatchValidator as SaplingBatchValidator, + Bundle as SaplingBundle, BundleAssembler as SaplingBundleAssembler, Prover, Verifier, }, streams::{ from_auto_file, from_buffered_file, from_data, from_hash_writer, from_size_computer, @@ -176,21 +179,26 @@ pub(crate) mod ffi { sighash_value: &[u8; 32], ) -> bool; - type BatchValidator; - fn init_batch_validator(cache_store: bool) -> Box; + #[cxx_name = "BatchValidator"] + type SaplingBatchValidator; + #[cxx_name = "init_batch_validator"] + fn init_sapling_batch_validator(cache_store: bool) -> Box; fn check_bundle( - self: &mut BatchValidator, + self: &mut SaplingBatchValidator, bundle: Box, sighash: [u8; 32], ) -> bool; - fn validate(self: &mut BatchValidator) -> bool; + fn validate(self: &mut SaplingBatchValidator) -> bool; } + unsafe extern "C++" { + include!("rust/orchard.h"); + type OrchardBundlePtr; + } #[namespace = "orchard_bundle"] extern "Rust" { type Action; type Bundle; - type OrchardBundle; fn cv(self: &Action) -> [u8; 32]; fn nullifier(self: &Action) -> [u8; 32]; @@ -201,7 +209,17 @@ pub(crate) mod ffi { fn out_ciphertext(self: &Action) -> [u8; 80]; fn spend_auth_sig(self: &Action) -> [u8; 64]; - unsafe fn from_tx_bundle(bundle: *const OrchardBundle) -> Box; + #[rust_name = "none_orchard_bundle"] + fn none() -> Box; + #[rust_name = "orchard_bundle_from_raw_box"] + unsafe fn from_raw_box(bundle: *mut OrchardBundlePtr) -> Box; + fn box_clone(self: &Bundle) -> Box; + #[rust_name = "parse_orchard_bundle"] + fn parse(stream: &mut CppStream<'_>) -> Result>; + fn serialize(self: &Bundle, stream: &mut CppStream<'_>) -> Result<()>; + fn as_ptr(self: &Bundle) -> *const OrchardBundlePtr; + fn recursive_dynamic_usage(self: &Bundle) -> usize; + fn is_present(self: &Bundle) -> bool; fn actions(self: &Bundle) -> Vec; fn num_actions(self: &Bundle) -> usize; fn enable_spends(self: &Bundle) -> bool; @@ -210,6 +228,17 @@ pub(crate) mod ffi { fn anchor(self: &Bundle) -> [u8; 32]; fn proof(self: &Bundle) -> Vec; fn binding_sig(self: &Bundle) -> [u8; 64]; + fn coinbase_outputs_are_valid(self: &Bundle) -> bool; + } + + #[namespace = "orchard"] + extern "Rust" { + #[cxx_name = "BatchValidator"] + type OrchardBatchValidator; + #[cxx_name = "init_batch_validator"] + fn orchard_batch_validation_init(cache_store: bool) -> Box; + fn add_bundle(self: &mut OrchardBatchValidator, bundle: Box, sighash: [u8; 32]); + fn validate(self: &mut OrchardBatchValidator) -> bool; } #[namespace = "merkle_frontier"] diff --git a/src/rust/src/orchard_bundle.rs b/src/rust/src/orchard_bundle.rs index 353ed0698..1ab0ace7d 100644 --- a/src/rust/src/orchard_bundle.rs +++ b/src/rust/src/orchard_bundle.rs @@ -1,8 +1,16 @@ +use std::{mem, ptr}; + +use memuse::DynamicUsage; use orchard::{ bundle::Authorized, + keys::OutgoingViewingKey, + note_encryption::OrchardDomain, primitives::redpallas::{Signature, SpendAuth}, }; -use zcash_primitives::transaction::components::Amount; +use zcash_note_encryption::try_output_recovery_with_ovk; +use zcash_primitives::transaction::components::{orchard as orchard_serialization, Amount}; + +use crate::{bridge::ffi, streams::CppStream}; pub struct Action(orchard::Action>); @@ -40,19 +48,83 @@ impl Action { } } +#[derive(Clone)] pub struct Bundle(Option>); -pub struct OrchardBundle; -pub(crate) unsafe fn from_tx_bundle(bundle: *const OrchardBundle) -> Box { - Box::new(Bundle( - { (bundle as *const orchard::Bundle).as_ref() }.cloned(), - )) + +pub(crate) fn none_orchard_bundle() -> Box { + Box::new(Bundle(None)) +} + +pub(crate) unsafe fn orchard_bundle_from_raw_box( + bundle: *mut ffi::OrchardBundlePtr, +) -> Box { + Bundle::from_raw_box(bundle) +} + +/// Parses an authorized Orchard bundle from the given stream. +pub(crate) fn parse_orchard_bundle(reader: &mut CppStream<'_>) -> Result, String> { + Bundle::parse(reader) } impl Bundle { + pub(crate) unsafe fn from_raw_box(bundle: *mut ffi::OrchardBundlePtr) -> Box { + Box::new(Bundle(if bundle.is_null() { + None + } else { + let bundle: *mut orchard::Bundle = bundle.cast(); + Some(*Box::from_raw(bundle)) + })) + } + + /// Returns a copy of the value. + pub(crate) fn box_clone(&self) -> Box { + Box::new(self.clone()) + } + + /// Parses an authorized Orchard bundle from the given stream. + pub(crate) fn parse(reader: &mut CppStream<'_>) -> Result, String> { + match orchard_serialization::read_v5_bundle(reader) { + Ok(parsed) => Ok(Box::new(Bundle(parsed))), + Err(e) => Err(format!("Failed to parse Orchard bundle: {}", e)), + } + } + + /// Serializes an authorized Orchard bundle to the given stream. + /// + /// If `bundle == None`, this serializes `nActionsOrchard = 0`. + pub(crate) fn serialize(&self, writer: &mut CppStream<'_>) -> Result<(), String> { + orchard_serialization::write_v5_bundle(self.inner(), writer) + .map_err(|e| format!("Failed to serialize Orchard bundle: {}", e)) + } + pub(crate) fn inner(&self) -> Option<&orchard::Bundle> { self.0.as_ref() } + pub(crate) fn as_ptr(&self) -> *const ffi::OrchardBundlePtr { + if let Some(bundle) = self.inner() { + let ret: *const orchard::Bundle = bundle; + ret.cast() + } else { + ptr::null() + } + } + + /// Returns the amount of dynamically-allocated memory used by this bundle. + pub(crate) fn recursive_dynamic_usage(&self) -> usize { + self.inner() + // Bundles are boxed on the heap, so we count their own size as well as the size + // of `Vec`s they allocate. + .map(|bundle| mem::size_of_val(bundle) + bundle.dynamic_usage()) + // If the transaction has no Orchard component, nothing is allocated for it. + .unwrap_or(0) + } + + /// Returns whether the Orchard bundle is present. + pub(crate) fn is_present(&self) -> bool { + self.0.is_some() + } + pub(crate) fn actions(&self) -> Vec { self.0 .iter() @@ -66,22 +138,36 @@ impl Bundle { self.inner().map(|b| b.actions().len()).unwrap_or(0) } + /// Returns whether the Orchard bundle is present and spends are enabled. pub(crate) fn enable_spends(&self) -> bool { self.inner() .map(|b| b.flags().spends_enabled()) .unwrap_or(false) } + /// Returns whether the Orchard bundle is present and outputs are enabled. pub(crate) fn enable_outputs(&self) -> bool { self.inner() .map(|b| b.flags().outputs_enabled()) .unwrap_or(false) } + /// Returns the value balance for this Orchard bundle. + /// + /// A transaction with no Orchard component has a value balance of zero. pub(crate) fn value_balance_zat(&self) -> i64 { - self.inner().map(|b| b.value_balance().into()).unwrap_or(0) + self.inner() + .map(|b| b.value_balance().into()) + // From section 7.1 of the Zcash prototol spec: + // If valueBalanceOrchard is not present, then v^balanceOrchard is defined to be 0. + .unwrap_or(0) } + /// Returns the anchor for the bundle. + /// + /// # Panics + /// + /// Panics if the bundle is not present. pub(crate) fn anchor(&self) -> [u8; 32] { self.inner() .expect("Bundle actions should have been checked to be non-empty") @@ -89,6 +175,11 @@ impl Bundle { .to_bytes() } + /// Returns the proof for the bundle. + /// + /// # Panics + /// + /// Panics if the bundle is not present. pub(crate) fn proof(&self) -> Vec { self.inner() .expect("Bundle actions should have been checked to be non-empty") @@ -98,6 +189,11 @@ impl Bundle { .to_vec() } + /// Returns the binding signature for the bundle. + /// + /// # Panics + /// + /// Panics if the bundle is not present. pub(crate) fn binding_sig(&self) -> [u8; 64] { self.inner() .expect("Bundle actions should have been checked to be non-empty") @@ -105,4 +201,33 @@ impl Bundle { .binding_signature() .into() } + + /// Returns whether all actions contained in the Orchard bundle can be decrypted with + /// the all-zeros OVK. + /// + /// Returns `true` if no Orchard actions are present. + /// + /// This should only be called on an Orchard bundle that is an element of a coinbase + /// transaction. + pub(crate) fn coinbase_outputs_are_valid(&self) -> bool { + if let Some(bundle) = self.inner() { + for act in bundle.actions() { + if try_output_recovery_with_ovk( + &OrchardDomain::for_action(act), + &OutgoingViewingKey::from([0u8; 32]), + act, + act.cv_net(), + &act.encrypted_note().out_ciphertext, + ) + .is_none() + { + return false; + } + } + } + + // Either there are no Orchard actions, or all of the outputs + // are decryptable with the all-zeros OVK. + true + } } diff --git a/src/rust/src/orchard_ffi.rs b/src/rust/src/orchard_ffi.rs index 0b13e74dc..7af27527d 100644 --- a/src/rust/src/orchard_ffi.rs +++ b/src/rust/src/orchard_ffi.rs @@ -1,301 +1,98 @@ -use std::{convert::TryInto, mem, ptr}; +use std::convert::TryInto; -use libc::size_t; -use memuse::DynamicUsage; -use orchard::{ - bundle::Authorized, keys::OutgoingViewingKey, note_encryption::OrchardDomain, Bundle, -}; use rand_core::OsRng; use tracing::{debug, error}; -use zcash_note_encryption::try_output_recovery_with_ovk; -use zcash_primitives::transaction::components::{orchard as orchard_serialization, Amount}; use crate::{ bundlecache::{orchard_bundle_validity_cache, orchard_bundle_validity_cache_mut, CacheEntries}, - streams_ffi::{CppStreamReader, CppStreamWriter, ReadCb, StreamObj, WriteCb}, + orchard_bundle::Bundle, }; -#[no_mangle] -pub extern "C" fn orchard_bundle_clone( - bundle: *const Bundle, -) -> *mut Bundle { - unsafe { bundle.as_ref() } - .map(|bundle| Box::into_raw(Box::new(bundle.clone()))) - .unwrap_or(std::ptr::null_mut()) -} - -#[no_mangle] -pub extern "C" fn orchard_bundle_free(bundle: *mut Bundle) { - if !bundle.is_null() { - drop(unsafe { Box::from_raw(bundle) }); - } -} - -#[no_mangle] -pub extern "C" fn orchard_bundle_recursive_dynamic_usage( - bundle: *const Bundle, -) -> size_t { - unsafe { bundle.as_ref() } - // Bundles are boxed on the heap, so we count their own size as well as the size - // of `Vec`s they allocate. - .map(|bundle| mem::size_of_val(bundle) + bundle.dynamic_usage()) - // If the transaction has no Orchard component, nothing is allocated for it. - .unwrap_or(0) -} - -#[no_mangle] -pub extern "C" fn orchard_bundle_parse( - stream: Option, - read_cb: Option, - bundle_ret: *mut *mut Bundle, -) -> bool { - let reader = CppStreamReader::from_raw_parts(stream, read_cb.unwrap()); - - match orchard_serialization::read_v5_bundle(reader) { - Ok(parsed) => { - unsafe { - *bundle_ret = if let Some(bundle) = parsed { - Box::into_raw(Box::new(bundle)) - } else { - ptr::null_mut::>() - }; - }; - true - } - Err(e) => { - error!("Failed to parse Orchard bundle: {}", e); - false - } - } -} - -#[no_mangle] -pub extern "C" fn orchard_bundle_serialize( - bundle: *const Bundle, - stream: Option, - write_cb: Option, -) -> bool { - let bundle = unsafe { bundle.as_ref() }; - let writer = CppStreamWriter::from_raw_parts(stream, write_cb.unwrap()); - - match orchard_serialization::write_v5_bundle(bundle, writer) { - Ok(()) => true, - Err(e) => { - error!("{}", e); - false - } - } -} - -#[no_mangle] - -pub extern "C" fn orchard_bundle_value_balance(bundle: *const Bundle) -> i64 { - unsafe { bundle.as_ref() } - .map(|bundle| (*bundle.value_balance()).into()) - // From section 7.1 of the Zcash prototol spec: - // If valueBalanceOrchard is not present, then v^balanceOrchard is defined to be 0. - .unwrap_or(0) -} - -#[no_mangle] -pub extern "C" fn orchard_bundle_actions_len(bundle: *const Bundle) -> usize { - if let Some(bundle) = unsafe { bundle.as_ref() } { - bundle.actions().len() - } else { - 0 - } -} - -#[no_mangle] -pub extern "C" fn orchard_bundle_nullifiers( - bundle: *const Bundle, - nullifiers_ret: *mut [u8; 32], - nullifiers_len: usize, -) -> bool { - if let Some(bundle) = unsafe { bundle.as_ref() } { - if nullifiers_len == bundle.actions().len() { - let res = unsafe { - assert!(!nullifiers_ret.is_null()); - std::slice::from_raw_parts_mut(nullifiers_ret, nullifiers_len) - }; - - for (action, nf_ret) in bundle.actions().iter().zip(res.iter_mut()) { - *nf_ret = action.nullifier().to_bytes(); - } - true - } else { - false - } - } else { - nullifiers_len == 0 - } -} - -#[no_mangle] -pub extern "C" fn orchard_bundle_anchor( - bundle: *const Bundle, - anchor_ret: *mut [u8; 32], -) -> bool { - if let Some((bundle, ret)) = unsafe { bundle.as_ref() }.zip(unsafe { anchor_ret.as_mut() }) { - ret.copy_from_slice(&bundle.anchor().to_bytes()); - - true - } else { - false - } -} - -pub struct BatchValidator { +struct BatchValidatorInner { validator: orchard::bundle::BatchValidator, queued_entries: CacheEntries, } +pub(crate) struct BatchValidator(Option); + /// Creates an Orchard bundle batch validation context. -/// -/// Please free this when you're done. -#[no_mangle] -pub extern "C" fn orchard_batch_validation_init(cache_store: bool) -> *mut BatchValidator { - let ctx = Box::new(BatchValidator { +pub(crate) fn orchard_batch_validation_init(cache_store: bool) -> Box { + Box::new(BatchValidator(Some(BatchValidatorInner { validator: orchard::bundle::BatchValidator::new(), queued_entries: CacheEntries::new(cache_store), - }); - Box::into_raw(ctx) + }))) } -/// Frees an Orchard bundle batch validation context returned from -/// [`orchard_batch_validation_init`]. -#[no_mangle] -pub extern "C" fn orchard_batch_validation_free(ctx: *mut BatchValidator) { - if !ctx.is_null() { - drop(unsafe { Box::from_raw(ctx) }); - } -} +impl BatchValidator { + /// Adds an Orchard bundle to this batch. + pub(crate) fn add_bundle(&mut self, bundle: Box, sighash: [u8; 32]) { + let batch = self.0.as_mut(); + let bundle = bundle.inner(); -/// Adds an Orchard bundle to this batch. -#[no_mangle] -pub extern "C" fn orchard_batch_add_bundle( - batch: *mut BatchValidator, - bundle: *const Bundle, - sighash: *const [u8; 32], -) { - let batch = unsafe { batch.as_mut() }; - let bundle = unsafe { bundle.as_ref() }; - let sighash = unsafe { sighash.as_ref() }; + match (batch, bundle) { + (Some(batch), Some(bundle)) => { + let cache = orchard_bundle_validity_cache(); - match (batch, bundle, sighash) { - (Some(batch), Some(bundle), Some(sighash)) => { - let cache = orchard_bundle_validity_cache(); + // Compute the cache entry for this bundle. + let cache_entry = { + let bundle_commitment = bundle.commitment(); + let bundle_authorizing_commitment = bundle.authorizing_commitment(); + cache.compute_entry( + bundle_commitment.0.as_bytes().try_into().unwrap(), + bundle_authorizing_commitment + .0 + .as_bytes() + .try_into() + .unwrap(), + &sighash, + ) + }; - // Compute the cache entry for this bundle. - let cache_entry = { - let bundle_commitment = bundle.commitment(); - let bundle_authorizing_commitment = bundle.authorizing_commitment(); - cache.compute_entry( - bundle_commitment.0.as_bytes().try_into().unwrap(), - bundle_authorizing_commitment - .0 - .as_bytes() - .try_into() - .unwrap(), - sighash, - ) - }; - - // Check if this bundle's validation result exists in the cache. - if !cache.contains(cache_entry, &mut batch.queued_entries) { - // The bundle has been added to `inner.queued_entries` because it was not - // in the cache. We now add its authorization to the validation batch. - batch.validator.add_bundle(bundle, *sighash); + // Check if this bundle's validation result exists in the cache. + if !cache.contains(cache_entry, &mut batch.queued_entries) { + // The bundle has been added to `inner.queued_entries` because it was not + // in the cache. We now add its authorization to the validation batch. + batch.validator.add_bundle(bundle, sighash); + } } + (Some(_), None) => debug!("Tx has no Orchard component"), + (None, _) => error!("orchard::BatchValidator has already been used"), } - (_, _, None) => error!("orchard_batch_add_bundle() called without sighash!"), - (Some(_), None, Some(_)) => debug!("Tx has no Orchard component"), - (None, Some(_), _) => debug!("Orchard BatchValidator not provided, assuming disabled."), - (None, None, _) => (), // Boring, don't bother logging. } -} -/// Validates this batch. -/// -/// - Returns `true` if `batch` is null. -/// - Returns `false` if any item in the batch is invalid. -/// -/// The batch validation context is freed by this function. -/// -/// ## Consensus rules -/// -/// [§4.6](https://zips.z.cash/protocol/protocol.pdf#actiondesc): -/// - Canonical element encodings are enforced by [`orchard_bundle_parse`]. -/// - SpendAuthSig^Orchard validity is enforced here. -/// - Proof validity is enforced here. -/// -/// [§7.1](https://zips.z.cash/protocol/protocol.pdf#txnencodingandconsensus): -/// - `bindingSigOrchard` validity is enforced here. -#[no_mangle] -pub extern "C" fn orchard_batch_validate(batch: *mut BatchValidator) -> bool { - if !batch.is_null() { - let batch = unsafe { Box::from_raw(batch) }; - let vk = unsafe { crate::ORCHARD_VK.as_ref() } - .expect("Parameters not loaded: ORCHARD_VK should have been initialized"); - if batch.validator.validate(vk, OsRng) { - // `BatchValidator::validate()` is only called if every - // `BatchValidator::check_bundle()` returned `true`, so at this point - // every bundle that was added to `inner.queued_entries` has valid - // authorization. - orchard_bundle_validity_cache_mut().insert(batch.queued_entries); - true + /// Validates this batch. + /// + /// - Returns `true` if `batch` is null. + /// - Returns `false` if any item in the batch is invalid. + /// + /// The batch validation context is freed by this function. + /// + /// ## Consensus rules + /// + /// [§4.6](https://zips.z.cash/protocol/protocol.pdf#actiondesc): + /// - Canonical element encodings are enforced by [`orchard_bundle_parse`]. + /// - SpendAuthSig^Orchard validity is enforced here. + /// - Proof validity is enforced here. + /// + /// [§7.1](https://zips.z.cash/protocol/protocol.pdf#txnencodingandconsensus): + /// - `bindingSigOrchard` validity is enforced here. + pub(crate) fn validate(&mut self) -> bool { + if let Some(inner) = self.0.take() { + let vk = unsafe { crate::ORCHARD_VK.as_ref() } + .expect("Parameters not loaded: ORCHARD_VK should have been initialized"); + if inner.validator.validate(vk, OsRng) { + // `BatchValidator::validate()` is only called if every + // `BatchValidator::check_bundle()` returned `true`, so at this point + // every bundle that was added to `inner.queued_entries` has valid + // authorization. + orchard_bundle_validity_cache_mut().insert(inner.queued_entries); + true + } else { + false + } } else { + error!("orchard::BatchValidator has already been used"); false } - } else { - // The orchard::BatchValidator C++ class uses null to represent a disabled batch - // validator. - debug!("Orchard BatchValidator not provided, assuming disabled."); - true } } - -#[no_mangle] -pub extern "C" fn orchard_bundle_outputs_enabled( - bundle: *const Bundle, -) -> bool { - let bundle = unsafe { bundle.as_ref() }; - bundle.map(|b| b.flags().outputs_enabled()).unwrap_or(false) -} - -#[no_mangle] -pub extern "C" fn orchard_bundle_spends_enabled(bundle: *const Bundle) -> bool { - let bundle = unsafe { bundle.as_ref() }; - bundle.map(|b| b.flags().spends_enabled()).unwrap_or(false) -} - -/// Returns whether all actions contained in the Orchard bundle -/// can be decrypted with the all-zeros OVK. Returns `true` -/// if no Orchard actions are present. -/// -/// This should only be called on an Orchard bundle that is -/// an element of a coinbase transaction. -#[no_mangle] -pub extern "C" fn orchard_bundle_coinbase_outputs_are_valid( - bundle: *const Bundle, -) -> bool { - if let Some(bundle) = unsafe { bundle.as_ref() } { - for act in bundle.actions() { - if try_output_recovery_with_ovk( - &OrchardDomain::for_action(act), - &OutgoingViewingKey::from([0u8; 32]), - act, - act.cv_net(), - &act.encrypted_note().out_ciphertext, - ) - .is_none() - { - return false; - } - } - } - - // Either there are no Orchard actions, or all of the outputs - // are decryptable with the all-zeros OVK. - true -} diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index 66a19bd88..dc65a53ee 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -356,7 +356,7 @@ void test_simple_joinsplit_invalidity(uint32_t consensusBranchId, CMutableTransa { auto verifier = ProofVerifier::Strict(); std::optional> saplingAuth = std::nullopt; - auto orchardAuth = orchard::AuthValidator::Disabled(); + std::optional> orchardAuth = std::nullopt; { // Ensure that empty vin/vout remain invalid without // joinsplits. diff --git a/src/wallet/orchard.h b/src/wallet/orchard.h index ce710f94d..85df41e9d 100644 --- a/src/wallet/orchard.h +++ b/src/wallet/orchard.h @@ -283,7 +283,7 @@ public: if (orchard_wallet_add_notes_from_bundle( inner.get(), tx.GetHash().begin(), - tx.GetOrchardBundle().inner.get(), + tx.GetOrchardBundle().inner->as_ptr(), &txMeta, PushOrchardActionIVK, PushSpendActionIdx @@ -310,7 +310,7 @@ public: return orchard_wallet_load_bundle( inner.get(), tx.GetHash().begin(), - tx.GetOrchardBundle().inner.get(), + tx.GetOrchardBundle().inner->as_ptr(), rawHints.data(), rawHints.size(), txMeta.vActionsSpendingMyNotes.data(), @@ -332,7 +332,7 @@ public: (uint32_t) nBlockHeight, txidx, tx.GetHash().begin(), - tx.GetOrchardBundle().inner.get() + tx.GetOrchardBundle().inner->as_ptr() )) { return false; } @@ -476,7 +476,7 @@ public: OrchardActions result; orchard_wallet_get_txdata( inner.get(), - tx.GetOrchardBundle().inner.get(), + tx.GetOrchardBundle().inner->as_ptr(), reinterpret_cast(ovks.data()), ovks.size(), &result, From ccbda94b30b43cf6997fafa93121fc0a47ad3f01 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Fri, 7 Apr 2023 12:17:00 +0000 Subject: [PATCH 2/2] gtest: Minor improvements to `CoinsTests` These were implemented while debugging the previous commit. --- src/gtest/test_coins.cpp | 48 ++++++++++++++++++++++++++-------------- 1 file changed, 32 insertions(+), 16 deletions(-) diff --git a/src/gtest/test_coins.cpp b/src/gtest/test_coins.cpp index ee9121e39..239ea82a9 100644 --- a/src/gtest/test_coins.cpp +++ b/src/gtest/test_coins.cpp @@ -302,7 +302,7 @@ public: uint256 dataToBeSigned; auto builder = orchard::Builder(true, true, orchardAnchor); mutableTx.orchardBundle = builder.Build().value().ProveAndSign({}, dataToBeSigned).value(); - orchardNullifier = mutableTx.orchardBundle.GetNullifiers()[0]; + orchardNullifier = mutableTx.orchardBundle.GetNullifiers().at(0); tx = CTransaction(mutableTx); } @@ -346,19 +346,19 @@ template<> bool GetAnchorAt(const CCoinsViewCacheTest &cache, const uint256 &rt, void checkNullifierCache(const CCoinsViewCacheTest &cache, const TxWithNullifiers &txWithNullifiers, bool shouldBeInCache) { // Make sure the nullifiers have not gotten mixed up - EXPECT_TRUE(!cache.GetNullifier(txWithNullifiers.sproutNullifier, SAPLING)); - EXPECT_TRUE(!cache.GetNullifier(txWithNullifiers.sproutNullifier, ORCHARD)); - EXPECT_TRUE(!cache.GetNullifier(txWithNullifiers.saplingNullifier, SPROUT)); - EXPECT_TRUE(!cache.GetNullifier(txWithNullifiers.saplingNullifier, ORCHARD)); - EXPECT_TRUE(!cache.GetNullifier(txWithNullifiers.orchardNullifier, SPROUT)); - EXPECT_TRUE(!cache.GetNullifier(txWithNullifiers.orchardNullifier, SAPLING)); + EXPECT_FALSE(cache.GetNullifier(txWithNullifiers.sproutNullifier, SAPLING)); + EXPECT_FALSE(cache.GetNullifier(txWithNullifiers.sproutNullifier, ORCHARD)); + EXPECT_FALSE(cache.GetNullifier(txWithNullifiers.saplingNullifier, SPROUT)); + EXPECT_FALSE(cache.GetNullifier(txWithNullifiers.saplingNullifier, ORCHARD)); + EXPECT_FALSE(cache.GetNullifier(txWithNullifiers.orchardNullifier, SPROUT)); + EXPECT_FALSE(cache.GetNullifier(txWithNullifiers.orchardNullifier, SAPLING)); // Check if the nullifiers either are or are not in the cache bool containsSproutNullifier = cache.GetNullifier(txWithNullifiers.sproutNullifier, SPROUT); bool containsSaplingNullifier = cache.GetNullifier(txWithNullifiers.saplingNullifier, SAPLING); bool containsOrchardNullifier = cache.GetNullifier(txWithNullifiers.orchardNullifier, ORCHARD); - EXPECT_TRUE(containsSproutNullifier == shouldBeInCache); - EXPECT_TRUE(containsSaplingNullifier == shouldBeInCache); - EXPECT_TRUE(containsOrchardNullifier == shouldBeInCache); + EXPECT_EQ(containsSproutNullifier, shouldBeInCache); + EXPECT_EQ(containsSaplingNullifier, shouldBeInCache); + EXPECT_EQ(containsOrchardNullifier, shouldBeInCache); } @@ -656,25 +656,41 @@ TEST(CoinsTests, AnchorRegression) TEST(CoinsTests, NullifiersTest) { + LoadProofParameters(); + CCoinsViewTest base; CCoinsViewCacheTest cache(&base); TxWithNullifiers txWithNullifiers; - checkNullifierCache(cache, txWithNullifiers, false); + { + SCOPED_TRACE("cache with unspent nullifiers"); + checkNullifierCache(cache, txWithNullifiers, false); + } cache.SetNullifiers(txWithNullifiers.tx, true); - checkNullifierCache(cache, txWithNullifiers, true); + { + SCOPED_TRACE("cache with spent nullifiers"); + checkNullifierCache(cache, txWithNullifiers, true); + } cache.Flush(); CCoinsViewCacheTest cache2(&base); - checkNullifierCache(cache2, txWithNullifiers, true); + { + SCOPED_TRACE("cache2 with spent nullifiers"); + checkNullifierCache(cache2, txWithNullifiers, true); + } cache2.SetNullifiers(txWithNullifiers.tx, false); - checkNullifierCache(cache2, txWithNullifiers, false); + { + SCOPED_TRACE("cache2 with unspent nullifiers"); + checkNullifierCache(cache2, txWithNullifiers, false); + } cache2.Flush(); CCoinsViewCacheTest cache3(&base); - - checkNullifierCache(cache3, txWithNullifiers, false); + { + SCOPED_TRACE("cache3 with unspent nullifiers"); + checkNullifierCache(cache3, txWithNullifiers, false); + } }