From ae9266a1cd078cd680b9590bd91327fb337d808e Mon Sep 17 00:00:00 2001 From: Janito Vaqueiro Ferreira Filho Date: Tue, 5 Oct 2021 21:27:33 +0000 Subject: [PATCH 1/9] Move `CurrentTxVersionInfo` into a new file Don't compile it for `zcash_script`. --- src/Makefile.am | 2 ++ src/primitives/transaction.cpp | 42 -------------------------- src/primitives/tx_version_info.cpp | 48 ++++++++++++++++++++++++++++++ 3 files changed, 50 insertions(+), 42 deletions(-) create mode 100644 src/primitives/tx_version_info.cpp diff --git a/src/Makefile.am b/src/Makefile.am index 034856579..9477cd033 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -393,6 +393,7 @@ libbitcoin_common_a_SOURCES = \ netbase.cpp \ primitives/block.cpp \ primitives/transaction.cpp \ + primitives/tx_version_info.cpp \ proof_verifier.cpp \ protocol.cpp \ pubkey.cpp \ @@ -558,6 +559,7 @@ libzcash_script_la_SOURCES = \ crypto/sha512.cpp \ hash.cpp \ primitives/transaction.cpp \ + primitives/tx_version_info.cpp \ pubkey.cpp \ script/zcash_script.cpp \ script/interpreter.cpp \ diff --git a/src/primitives/transaction.cpp b/src/primitives/transaction.cpp index 8239cfc98..2263ab91b 100644 --- a/src/primitives/transaction.cpp +++ b/src/primitives/transaction.cpp @@ -425,45 +425,3 @@ std::string CTransaction::ToString() const str += " " + vout[i].ToString() + "\n"; return str; } - -/** - * Returns the most recent supported transaction version and version group id, - * as of the specified activation height and active features. - */ -TxVersionInfo CurrentTxVersionInfo( - const Consensus::Params& consensus, - int nHeight, - bool requireSprout) -{ - if (consensus.NetworkUpgradeActive(nHeight, Consensus::UPGRADE_ZFUTURE)) { - return { - .fOverwintered = true, - .nVersionGroupId = ZFUTURE_VERSION_GROUP_ID, - .nVersion = ZFUTURE_TX_VERSION - }; - } else if (consensus.NetworkUpgradeActive(nHeight, Consensus::UPGRADE_NU5) && !requireSprout) { - return { - .fOverwintered = true, - .nVersionGroupId = ZIP225_VERSION_GROUP_ID, - .nVersion = ZIP225_TX_VERSION - }; - } else if (consensus.NetworkUpgradeActive(nHeight, Consensus::UPGRADE_SAPLING)) { - return { - .fOverwintered = true, - .nVersionGroupId = SAPLING_VERSION_GROUP_ID, - .nVersion = SAPLING_TX_VERSION - }; - } else if (consensus.NetworkUpgradeActive(nHeight, Consensus::UPGRADE_OVERWINTER)) { - return { - .fOverwintered = true, - .nVersionGroupId = OVERWINTER_VERSION_GROUP_ID, - .nVersion = OVERWINTER_TX_VERSION - }; - } else { - return { - .fOverwintered = false, - .nVersionGroupId = 0, - .nVersion = CTransaction::SPROUT_MIN_CURRENT_VERSION - }; - } -} diff --git a/src/primitives/tx_version_info.cpp b/src/primitives/tx_version_info.cpp new file mode 100644 index 000000000..80b59ac10 --- /dev/null +++ b/src/primitives/tx_version_info.cpp @@ -0,0 +1,48 @@ +// Copyright (c) 2009-2010 Satoshi Nakamoto +// Copyright (c) 2009-2014 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or https://www.opensource.org/licenses/mit-license.php . + +#include "primitives/transaction.h" + +/** + * Returns the most recent supported transaction version and version group id, + * as of the specified activation height and active features. + */ +TxVersionInfo CurrentTxVersionInfo( + const Consensus::Params& consensus, + int nHeight, + bool requireSprout) +{ + if (consensus.NetworkUpgradeActive(nHeight, Consensus::UPGRADE_ZFUTURE)) { + return { + .fOverwintered = true, + .nVersionGroupId = ZFUTURE_VERSION_GROUP_ID, + .nVersion = ZFUTURE_TX_VERSION + }; + } else if (consensus.NetworkUpgradeActive(nHeight, Consensus::UPGRADE_NU5) && !requireSprout) { + return { + .fOverwintered = true, + .nVersionGroupId = ZIP225_VERSION_GROUP_ID, + .nVersion = ZIP225_TX_VERSION + }; + } else if (consensus.NetworkUpgradeActive(nHeight, Consensus::UPGRADE_SAPLING)) { + return { + .fOverwintered = true, + .nVersionGroupId = SAPLING_VERSION_GROUP_ID, + .nVersion = SAPLING_TX_VERSION + }; + } else if (consensus.NetworkUpgradeActive(nHeight, Consensus::UPGRADE_OVERWINTER)) { + return { + .fOverwintered = true, + .nVersionGroupId = OVERWINTER_VERSION_GROUP_ID, + .nVersion = OVERWINTER_TX_VERSION + }; + } else { + return { + .fOverwintered = false, + .nVersionGroupId = 0, + .nVersion = CTransaction::SPROUT_MIN_CURRENT_VERSION + }; + } +} From 31bd2ba4f1f85814c34c252a198906875c8df8cb Mon Sep 17 00:00:00 2001 From: teor Date: Tue, 9 Nov 2021 12:45:36 +1000 Subject: [PATCH 2/9] Add sigop count functions to zcash_script library --- src/script/zcash_script.cpp | 55 ++++++++++++++++++++++++++++++++++++- src/script/zcash_script.h | 21 ++++++++++++++ 2 files changed, 75 insertions(+), 1 deletion(-) diff --git a/src/script/zcash_script.cpp b/src/script/zcash_script.cpp index 9ea67fcc5..a60ea7890 100644 --- a/src/script/zcash_script.cpp +++ b/src/script/zcash_script.cpp @@ -5,6 +5,7 @@ #include "zcash_script.h" +#include "consensus/consensus.h" #include "consensus/upgrades.h" #include "primitives/transaction.h" #include "pubkey.h" @@ -69,6 +70,21 @@ struct ECCryptoClosure }; ECCryptoClosure instance_of_eccryptoclosure; + +/// Copy of GetLegacySigOpCount from main.cpp commit c4b2ef7c4 +unsigned int GetLegacySigOpCount(const CTransaction& tx) +{ + unsigned int nSigOps = 0; + for (const CTxIn& txin : tx.vin) + { + nSigOps += txin.scriptSig.GetSigOpCount(false); + } + for (const CTxOut& txout : tx.vout) + { + nSigOps += txout.scriptPubKey.GetSigOpCount(false); + } + return nSigOps; +} } struct PrecomputedTransaction { @@ -92,7 +108,7 @@ void* zcash_script_new_precomputed_tx( return nullptr; } - // Regardless of the verification result, the tx did not error. + // Deserializing the tx did not error. set_error(err, zcash_script_ERR_OK); auto preTx = new PrecomputedTransaction(tx); return preTx; @@ -166,6 +182,43 @@ int zcash_script_verify( } } +unsigned int zcash_script_legacy_sigop_count_precomputed( + const void* pre_preTx, + zcash_script_error* err) +{ + const PrecomputedTransaction* preTx = static_cast(pre_preTx); + const CTransaction& tx = preTx->tx; + + // The current implementation of this method never errors. + set_error(err, zcash_script_ERR_OK); + + return GetLegacySigOpCount(preTx->tx); +} + +unsigned int zcash_script_legacy_sigop_count( + const unsigned char *txTo, + unsigned int txToLen, + zcash_script_error* err) +{ + try { + TxInputStream stream(SER_NETWORK, PROTOCOL_VERSION, txTo, txToLen); + CTransaction tx; + stream >> tx; + if (GetSerializeSize(tx, SER_NETWORK, PROTOCOL_VERSION) != txToLen) { + set_error(err, zcash_script_ERR_TX_SIZE_MISMATCH); + return UINT_MAX; + } + + // Deserializing the tx did not error. + set_error(err, zcash_script_ERR_OK); + + return GetLegacySigOpCount(tx); + } catch (const std::exception&) { + set_error(err, zcash_script_ERR_TX_DESERIALIZE); // Error deserializing + return UINT_MAX; + } +} + unsigned int zcash_script_version() { // Just use the API version for now diff --git a/src/script/zcash_script.h b/src/script/zcash_script.h index a34963cba..12c8e06cb 100644 --- a/src/script/zcash_script.h +++ b/src/script/zcash_script.h @@ -99,6 +99,27 @@ EXPORT_SYMBOL int zcash_script_verify( uint32_t consensusBranchId, zcash_script_error* err); +/// Returns the number of transparent signature operations in the +/// transparent inputs and outputs of the precomputed transaction +/// pointed to by preTx. +/// +/// Returns UINT_MAX on error. +/// If not NULL, err will contain an error/success code for the operation. +EXPORT_SYMBOL unsigned int zcash_script_legacy_sigop_count_precomputed( + const void* preTx, + zcash_script_error* err); + +/// Returns the number of transparent signature operations in the +/// transparent inputs and outputs of the serialized transaction +/// pointed to by txTo. +/// +/// Returns UINT_MAX on error. +/// If not NULL, err will contain an error/success code for the operation. +EXPORT_SYMBOL unsigned int zcash_script_legacy_sigop_count( + const unsigned char *txTo, + unsigned int txToLen, + zcash_script_error* err); + EXPORT_SYMBOL unsigned int zcash_script_version(); #ifdef __cplusplus From fa181a2d08a310861b344af97fae6d5b3392d07a Mon Sep 17 00:00:00 2001 From: teor Date: Tue, 9 Nov 2021 12:51:57 +1000 Subject: [PATCH 3/9] Increment the zcash_script API version --- src/script/zcash_script.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/script/zcash_script.h b/src/script/zcash_script.h index 12c8e06cb..cecca4e75 100644 --- a/src/script/zcash_script.h +++ b/src/script/zcash_script.h @@ -33,7 +33,7 @@ extern "C" { #endif -#define ZCASH_SCRIPT_API_VER 1 +#define ZCASH_SCRIPT_API_VER 2 typedef enum zcash_script_error_t { @@ -120,6 +120,7 @@ EXPORT_SYMBOL unsigned int zcash_script_legacy_sigop_count( unsigned int txToLen, zcash_script_error* err); +/// Returns the current version of the zcash_script library. EXPORT_SYMBOL unsigned int zcash_script_version(); #ifdef __cplusplus From 77ab1831a1918ef6039b7569f658c83b6342718c Mon Sep 17 00:00:00 2001 From: teor Date: Tue, 9 Nov 2021 12:58:54 +1000 Subject: [PATCH 4/9] Remove an unused header --- src/script/zcash_script.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/script/zcash_script.cpp b/src/script/zcash_script.cpp index a60ea7890..2c57c86ae 100644 --- a/src/script/zcash_script.cpp +++ b/src/script/zcash_script.cpp @@ -5,7 +5,6 @@ #include "zcash_script.h" -#include "consensus/consensus.h" #include "consensus/upgrades.h" #include "primitives/transaction.h" #include "pubkey.h" From 3bae89c73202d6b61fd4f397355f7877167a2416 Mon Sep 17 00:00:00 2001 From: teor Date: Tue, 16 Nov 2021 10:46:08 +1000 Subject: [PATCH 5/9] Use correct copyright header Co-authored-by: str4d --- src/primitives/tx_version_info.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/primitives/tx_version_info.cpp b/src/primitives/tx_version_info.cpp index 80b59ac10..bdcabcb1e 100644 --- a/src/primitives/tx_version_info.cpp +++ b/src/primitives/tx_version_info.cpp @@ -1,5 +1,4 @@ -// Copyright (c) 2009-2010 Satoshi Nakamoto -// Copyright (c) 2009-2014 The Bitcoin Core developers +// Copyright (c) 2021 The Zcash developers // Distributed under the MIT software license, see the accompanying // file COPYING or https://www.opensource.org/licenses/mit-license.php . From 7d24ada7579d8a6f912c53f1b86cc5a9fe4483c1 Mon Sep 17 00:00:00 2001 From: teor Date: Tue, 16 Nov 2021 10:47:02 +1000 Subject: [PATCH 6/9] Remove redundant variable Co-authored-by: str4d --- src/script/zcash_script.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/script/zcash_script.cpp b/src/script/zcash_script.cpp index 2c57c86ae..d6b425a7a 100644 --- a/src/script/zcash_script.cpp +++ b/src/script/zcash_script.cpp @@ -186,7 +186,6 @@ unsigned int zcash_script_legacy_sigop_count_precomputed( zcash_script_error* err) { const PrecomputedTransaction* preTx = static_cast(pre_preTx); - const CTransaction& tx = preTx->tx; // The current implementation of this method never errors. set_error(err, zcash_script_ERR_OK); From f4a8dc57e0201e8da21a6b32b82f35c54433e1be Mon Sep 17 00:00:00 2001 From: teor Date: Tue, 16 Nov 2021 10:49:37 +1000 Subject: [PATCH 7/9] Explain UINT_MAX error return value --- src/script/zcash_script.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/script/zcash_script.h b/src/script/zcash_script.h index cecca4e75..d2c52b0ea 100644 --- a/src/script/zcash_script.h +++ b/src/script/zcash_script.h @@ -103,7 +103,7 @@ EXPORT_SYMBOL int zcash_script_verify( /// transparent inputs and outputs of the precomputed transaction /// pointed to by preTx. /// -/// Returns UINT_MAX on error. +/// Returns UINT_MAX on error, so that invalid transactions don't pass the Zcash consensus rules. /// If not NULL, err will contain an error/success code for the operation. EXPORT_SYMBOL unsigned int zcash_script_legacy_sigop_count_precomputed( const void* preTx, From bdbd86edb7f10cf77edb281467c8cbe1af270a8d Mon Sep 17 00:00:00 2001 From: teor Date: Tue, 16 Nov 2021 10:51:07 +1000 Subject: [PATCH 8/9] Explain how to get rid of a tiny duplicated function --- src/script/zcash_script.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/script/zcash_script.cpp b/src/script/zcash_script.cpp index d6b425a7a..aed1e99a9 100644 --- a/src/script/zcash_script.cpp +++ b/src/script/zcash_script.cpp @@ -70,7 +70,8 @@ struct ECCryptoClosure ECCryptoClosure instance_of_eccryptoclosure; -/// Copy of GetLegacySigOpCount from main.cpp commit c4b2ef7c4 +// Copy of GetLegacySigOpCount from main.cpp commit c4b2ef7c4. +// Replace with the copy from src/consensus/tx_verify.{cpp,h} after backporting that refactor. unsigned int GetLegacySigOpCount(const CTransaction& tx) { unsigned int nSigOps = 0; From 675e489aff8b02242b8ff1df371164ad5aacbd23 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Tue, 16 Nov 2021 18:29:50 +0000 Subject: [PATCH 9/9] rust: Move `incremental_sinsemilla_tree_ffi` into crate root Having this be a submodule of `orchard_ffi` while following Rust 2018 module structure made it impossible to use the `include!` macro on `orchard_ffi.rs`, which is exactly what the `zcash_script` crate does. See this comment for details: https://github.com/rust-lang/rust/issues/50132#issuecomment-969450868 To resolve this, we restructure the FFI library crate to only have FFI methods in "leaf" module files. --- .../src/{orchard_ffi => }/incremental_sinsemilla_tree_ffi.rs | 4 ++-- src/rust/src/orchard_ffi.rs | 2 -- src/rust/src/rustzcash.rs | 1 + 3 files changed, 3 insertions(+), 4 deletions(-) rename src/rust/src/{orchard_ffi => }/incremental_sinsemilla_tree_ffi.rs (98%) diff --git a/src/rust/src/orchard_ffi/incremental_sinsemilla_tree_ffi.rs b/src/rust/src/incremental_sinsemilla_tree_ffi.rs similarity index 98% rename from src/rust/src/orchard_ffi/incremental_sinsemilla_tree_ffi.rs rename to src/rust/src/incremental_sinsemilla_tree_ffi.rs index 370ae521c..37fc07c78 100644 --- a/src/rust/src/orchard_ffi/incremental_sinsemilla_tree_ffi.rs +++ b/src/rust/src/incremental_sinsemilla_tree_ffi.rs @@ -6,13 +6,13 @@ use std::mem::size_of_val; use std::ptr; use orchard::{bundle::Authorized, tree::MerkleHashOrchard}; - +use tracing::error; use zcash_primitives::{ merkle_tree::incremental::{read_frontier_v1, read_tree, write_frontier_v1, write_tree}, transaction::components::Amount, }; -use crate::orchard_ffi::{error, CppStreamReader, CppStreamWriter, ReadCb, StreamObj, WriteCb}; +use crate::streams_ffi::{CppStreamReader, CppStreamWriter, ReadCb, StreamObj, WriteCb}; pub const MERKLE_DEPTH: u8 = 32; pub const MAX_CHECKPOINTS: usize = 100; diff --git a/src/rust/src/orchard_ffi.rs b/src/rust/src/orchard_ffi.rs index 7bfea4080..dbc6649c6 100644 --- a/src/rust/src/orchard_ffi.rs +++ b/src/rust/src/orchard_ffi.rs @@ -19,8 +19,6 @@ use zcash_primitives::transaction::{ use crate::streams_ffi::{CppStreamReader, CppStreamWriter, ReadCb, StreamObj, WriteCb}; -mod incremental_sinsemilla_tree_ffi; - #[no_mangle] pub extern "C" fn orchard_bundle_clone( bundle: *const Bundle, diff --git a/src/rust/src/rustzcash.rs b/src/rust/src/rustzcash.rs index c4b1e6125..e4879d0c4 100644 --- a/src/rust/src/rustzcash.rs +++ b/src/rust/src/rustzcash.rs @@ -70,6 +70,7 @@ mod tracing_ffi; mod address_ffi; mod history_ffi; +mod incremental_sinsemilla_tree_ffi; mod orchard_ffi; mod transaction_ffi; mod zip339_ffi;