From cfed38503ed015c978d645b0218e2d5ca8ed3216 Mon Sep 17 00:00:00 2001 From: Glenn Willen Date: Tue, 12 Oct 2021 12:23:11 -0700 Subject: [PATCH 01/12] Make error messages when talking to parent chain daemon more useful. --- src/mainchainrpc.cpp | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/mainchainrpc.cpp b/src/mainchainrpc.cpp index ef7725bbe4..6018dd8de6 100644 --- a/src/mainchainrpc.cpp +++ b/src/mainchainrpc.cpp @@ -158,19 +158,20 @@ bool IsConfirmedBitcoinBlock(const uint256& hash, const int nMinConfirmationDept UniValue params(UniValue::VARR); params.push_back(hash.GetHex()); UniValue reply = CallMainChainRPC("getblockheader", params); - if (!find_value(reply, "error").isNull()) { - LogPrintf("ERROR: Got error reply from bitcoind getblockheader.\n"); + UniValue errval = find_value(reply, "error"); + if (!errval.isNull()) { + LogPrintf("WARNING: Got error reply from bitcoind getblockheader: %s\n", errval.write()); return false; } UniValue result = find_value(reply, "result"); if (!result.isObject()) { - LogPrintf("ERROR: bitcoind getblockheader result was malformed (not object).\n"); + LogPrintf("ERROR: bitcoind getblockheader result was malformed (not object): %s\n", result.write()); return false; } UniValue confirmations = find_value(result.get_obj(), "confirmations"); if (!confirmations.isNum() || confirmations.get_int64() < nMinConfirmationDepth) { - LogPrintf("Insufficient confirmations (got %s).\n", confirmations.write()); + LogPrintf("Insufficient confirmations (got %s, need at least %d).\n", confirmations.write(), nMinConfirmationDepth); return false; } @@ -178,16 +179,16 @@ bool IsConfirmedBitcoinBlock(const uint256& hash, const int nMinConfirmationDept if (nbTxs != 0) { UniValue nTx = find_value(result.get_obj(), "nTx"); if (!nTx.isNum() || nTx.get_int64() != nbTxs) { - LogPrintf("ERROR: Invalid number of transactions in merkle block for %s\n", - hash.GetHex()); + LogPrintf("ERROR: Invalid number of transactions in merkle block for %s (got %s, need exactly %d)\n", + hash.GetHex(), nTx.write(), nbTxs); return false; } } } catch (CConnectionFailed& e) { - LogPrintf("ERROR: Lost connection to mainchain daemon RPC, you will want to restart after fixing this!\n"); + LogPrintf("WARNING: Lost connection to mainchain daemon RPC; will retry.\n"); return false; } catch (...) { - LogPrintf("ERROR: Failure connecting to mainchain daemon RPC, you will want to restart after fixing this!\n"); + LogPrintf("WARNING: Failure connecting to mainchain daemon RPC; will retry.\n"); return false; } return true; From 6c1d0d3ca80fe1a9d14f2559d11ba9e89f45fd72 Mon Sep 17 00:00:00 2001 From: Jeff Frontz Date: Thu, 11 Nov 2021 13:39:10 -0500 Subject: [PATCH 02/12] Re-evaluate peg-out descriptor if wallet is reparsed --- src/wallet/rpcwallet.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index b234c37d02..43da2be9fb 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -5656,7 +5656,7 @@ static RPCHelpMan sendtomainchain_pak() FlatSigningProvider provider; std::string error; - const auto descriptor = Parse(pwallet->offline_desc, provider, error); + auto descriptor = Parse(pwallet->offline_desc, provider, error); LegacyScriptPubKeyMan* spk_man = pwallet->GetLegacyScriptPubKeyMan(); if (!spk_man) { @@ -5669,6 +5669,11 @@ static RPCHelpMan sendtomainchain_pak() if (!pwallet->SetOfflineDescriptor(offline_desc)) { throw JSONRPCError(RPC_WALLET_ERROR, "Couldn't set wallet descriptor for peg-outs."); } + + descriptor = Parse(pwallet->offline_desc, provider, error); + if (!descriptor) { + throw JSONRPCError(RPC_WALLET_ERROR, "descriptor still null. This is a bug in elementsd."); + } } std::string desc_str = pwallet->offline_desc; From 3c911a7ed7115e654228fb816c55595d62559c4a Mon Sep 17 00:00:00 2001 From: Riccardo Casatta Date: Mon, 15 Nov 2021 12:16:15 +0100 Subject: [PATCH 03/12] Typos in pset doc --- doc/pset.mediawiki | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/doc/pset.mediawiki b/doc/pset.mediawiki index bc04590e06..54428fc83d 100644 --- a/doc/pset.mediawiki +++ b/doc/pset.mediawiki @@ -24,7 +24,7 @@ This BIP is licensed under the 2-clause BSD license. ==Specification== -The Partially Signed ELements Transaction (PSET) format is identical to the BIP 370 PSBT format. +The Partially Signed Elements Transaction (PSET) format is identical to the BIP 370 PSBT format. The changes are new proprietary type fields, a new magic sequence, and new roles. The fields added for PSET are only allowed when the PSBT version is 2. @@ -60,13 +60,13 @@ The currently defined elements global proprietary types are as follows: | None | No key data | <8-bit uint> -| An 8 bit little endian unsigned integer as a bitfield for various elements specific transaction modification flags. Bit 0 is the PSBT Blinded flag and it is set to 1 to indicate that the PSET has not been blinded yet. Once all confidential values, rangeproofs, and asset surjection proofs have been attached to the PSET, it must be set to 0. +| An 8 bit little endian unsigned integer as a bitfield for various elements specific transaction modification flags. Bit 0 is the PSET Blinded flag and it is set to 1 to indicate that the PSET has not been blinded yet. Once all confidential values, rangeproofs, and asset surjection proofs have been attached to the PSET, it must be set to 0. | | 0 | 2 |} -The currently defined elements per-input proprietary types are as folows: +The currently defined elements per-input proprietary types are as follows: {| ! Name @@ -213,7 +213,7 @@ The currently defined elements per-input proprietary types are as folows: | PSBT_ELEMENTS_IN_ISSUANCE_ASSET_ENTROPY = 0x0d | None | No key data -| <32 byte entrpy> +| <32 byte entropy> | The 32 byte asset entropy. For new issuances, an arbitrary and optional 32 bytes of no consensus meaning combined used as additional entropy in the asset tag calculation. For reissuances, the original, final entropy used for the asset tag calculation. | | 0 @@ -423,7 +423,7 @@ It will also add the ephemeral pubkey used for ECDH of the nonce for the rangepr The blinder will then compute a scalar offset that will be added as a PSBT_ELEMENTS_GLOBAL_SCALAR. For each input and output owned/blinded by this blinder, the following formula is computed: - asset_blinding_factor * amount + amount_blinding_factor (mod n). + asset_blinding_factor * amount + amount_blinding_factor (mod n). The scalars for the inputs are summed, and then that sum is subtracted from the sum of the scalars for the outputs. The result is the scalar offset added as a PSBT_ELEMENTS_GLOBAL_SCALAR. @@ -441,7 +441,7 @@ A single entity is likely to be a Creator, Updater, and Blinder. ===Signer=== -In addition to the BIP 370 PSBT Signer behavior, PSET specifies some addtional constraints. +In addition to the BIP 370 PSBT Signer behavior, PSET specifies some additional constraints. Before signing, the Signer must check whether blinding is complete. If any output contains a blinding pubkey but no commitments or proofs, then it must not sign. ===Combiner=== @@ -470,4 +470,4 @@ TBD ==Reference implementation== -The reference implementation of the PSBT format is available at https://github.com/achow101/elements/tree/pset. +The reference implementation of the PSET format is available at https://github.com/achow101/elements/tree/pset. From cdfb4c9c6cd8909efeb59c8c47cdcef7c1822b26 Mon Sep 17 00:00:00 2001 From: Glenn Willen Date: Tue, 9 Nov 2021 21:06:42 -0800 Subject: [PATCH 04/12] Fix elements multiple-header-download issue. This fixes an issue which causes Elements to download the blockchain headers multiple times during initial block download. In particular: each time we receive an INV P2P message with a new block (about once a minute), we start downloading the headers, again, in parallel with any existing download(s) in progress. With this change, after we receive each batch of headers, we check whether any of the headers in it were new to us. If not (they were all duplicates), we stop there, and do not ask the peer for another batch. This reduces the maximum amount of duplication to about 2x, which is not ideal, but a HUGE improvement. --- src/net_processing.cpp | 6 ++++-- src/validation.cpp | 19 ++++++++++++++++--- src/validation.h | 5 +++-- 3 files changed, 23 insertions(+), 7 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index c649cf7757..042b58474a 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1843,7 +1843,8 @@ void PeerManager::ProcessHeadersMessage(CNode& pfrom, const std::vectorm_last_block_announcement = GetTime(); } - if (nCount == MAX_HEADERS_RESULTS) { + if (nCount == MAX_HEADERS_RESULTS && !all_duplicate) { // Headers message had its maximum size; the peer may have more headers. // TODO: optimize: if pindexLast is an ancestor of ::ChainActive().Tip or pindexBestHeader, continue // from there instead. + // HOWEVER, if all headers we got this time were duplicates that we already had, don't ask for any more. LogPrint(BCLog::NET, "more getheaders (%d) to end to peer=%d (startheight:%d)\n", pindexLast->nHeight, pfrom.GetId(), pfrom.nStartingHeight); m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETHEADERS, ::ChainActive().GetLocator(pindexLast), uint256())); } diff --git a/src/validation.cpp b/src/validation.cpp index 24605b7bb6..ff2f81c5d8 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -4036,16 +4036,22 @@ static bool ContextualCheckBlock(const CBlock& block, BlockValidationState& stat return true; } -bool BlockManager::AcceptBlockHeader(const CBlockHeader& block, BlockValidationState& state, const CChainParams& chainparams, CBlockIndex** ppindex) +bool BlockManager::AcceptBlockHeader(const CBlockHeader& block, BlockValidationState& state, const CChainParams& chainparams, CBlockIndex** ppindex, bool* duplicate) { AssertLockHeld(cs_main); // Check for duplicate uint256 hash = block.GetHash(); BlockMap::iterator miSelf = m_block_index.find(hash); CBlockIndex *pindex = nullptr; + if (duplicate) { + *duplicate = false; + } if (hash != chainparams.GetConsensus().hashGenesisBlock) { if (miSelf != m_block_index.end()) { // Block header is already known. + if (duplicate) { + *duplicate = true; + } pindex = miSelf->second; if (ppindex) *ppindex = pindex; @@ -4125,17 +4131,24 @@ bool BlockManager::AcceptBlockHeader(const CBlockHeader& block, BlockValidationS } // Exposed wrapper for AcceptBlockHeader -bool ChainstateManager::ProcessNewBlockHeaders(const std::vector& headers, BlockValidationState& state, const CChainParams& chainparams, const CBlockIndex** ppindex) +bool ChainstateManager::ProcessNewBlockHeaders(const std::vector& headers, BlockValidationState& state, const CChainParams& chainparams, const CBlockIndex** ppindex, bool* all_duplicate) { AssertLockNotHeld(cs_main); { LOCK(cs_main); + if (all_duplicate) { + *all_duplicate = true; + } + bool duplicate = false; for (const CBlockHeader& header : headers) { CBlockIndex *pindex = nullptr; // Use a temp pindex instead of ppindex to avoid a const_cast bool accepted = m_blockman.AcceptBlockHeader( - header, state, chainparams, &pindex); + header, state, chainparams, &pindex, &duplicate); ::ChainstateActive().CheckBlockIndex(chainparams.GetConsensus()); + if (all_duplicate) { + (*all_duplicate) &= duplicate; // False if any are false + } if (!accepted) { return false; } diff --git a/src/validation.h b/src/validation.h index f1891be4cb..174a19eac7 100644 --- a/src/validation.h +++ b/src/validation.h @@ -440,7 +440,8 @@ public: const CBlockHeader& block, BlockValidationState& state, const CChainParams& chainparams, - CBlockIndex** ppindex) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + CBlockIndex** ppindex, + bool* duplicate = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs_main); ~BlockManager() { Unload(); @@ -926,7 +927,7 @@ public: * @param[in] chainparams The params for the chain we want to connect to * @param[out] ppindex If set, the pointer will be set to point to the last new block index object for the given headers */ - bool ProcessNewBlockHeaders(const std::vector& block, BlockValidationState& state, const CChainParams& chainparams, const CBlockIndex** ppindex = nullptr) LOCKS_EXCLUDED(cs_main); + bool ProcessNewBlockHeaders(const std::vector& block, BlockValidationState& state, const CChainParams& chainparams, const CBlockIndex** ppindex = nullptr, bool* all_duplicate = nullptr) LOCKS_EXCLUDED(cs_main); //! Load the block tree and coins database from disk, initializing state if we're running with -reindex bool LoadBlockIndex(const CChainParams& chainparams) EXCLUSIVE_LOCKS_REQUIRED(cs_main); From e72544da903ea2c79f6ed0276c96d2fb9e782c9a Mon Sep 17 00:00:00 2001 From: Glenn Willen Date: Wed, 17 Nov 2021 17:02:14 -0800 Subject: [PATCH 05/12] Mark pegin-related args to createrawtransaction as optional. --- src/rpc/rawtransaction.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index f60953eee0..b8cfd1eaae 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -405,9 +405,9 @@ static RPCHelpMan createrawtransaction() {"txid", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The transaction id"}, {"vout", RPCArg::Type::NUM, RPCArg::Optional::NO, "The output number"}, {"sequence", RPCArg::Type::NUM, /* default */ "depends on the value of the 'replaceable' and 'locktime' arguments", "The sequence number"}, - {"pegin_bitcoin_tx", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The raw bitcoin transaction (in hex) depositing bitcoin to the mainchain_address generated by getpeginaddress"}, - {"pegin_txout_proof", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "A rawtxoutproof (in hex) generated by the mainchain daemon's `gettxoutproof` containing a proof of only bitcoin_tx"}, - {"pegin_claim_script", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The claim script generated by getpeginaddress."}, + {"pegin_bitcoin_tx", RPCArg::Type::STR_HEX, RPCArg::Optional::OMITTED_NAMED_ARG, "(only for pegin inputs) The raw bitcoin transaction (in hex) depositing bitcoin to the mainchain_address generated by getpeginaddress"}, + {"pegin_txout_proof", RPCArg::Type::STR_HEX, RPCArg::Optional::OMITTED_NAMED_ARG, "(only for pegin inputs) A rawtxoutproof (in hex) generated by the mainchain daemon's `gettxoutproof` containing a proof of only bitcoin_tx"}, + {"pegin_claim_script", RPCArg::Type::STR_HEX, RPCArg::Optional::OMITTED_NAMED_ARG, "(only for pegin inputs) The claim script generated by getpeginaddress."}, }, }, }, From b2dd758ce4b9fbff88205c49ef6392862b15d6d1 Mon Sep 17 00:00:00 2001 From: Glenn Willen Date: Mon, 22 Nov 2021 13:37:31 -0800 Subject: [PATCH 06/12] Remove redundant log message in IsValidPeginWitness --- src/pegins.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/pegins.cpp b/src/pegins.cpp index 6bb9229ebe..2ba435623b 100644 --- a/src/pegins.cpp +++ b/src/pegins.cpp @@ -373,7 +373,6 @@ bool IsValidPeginWitness(const CScriptWitness& pegin_witness, const std::vector< if (tx_index == 0) { required_depth = std::max(required_depth, (unsigned int)COINBASE_MATURITY); } - LogPrintf("Required depth: %d\n", required_depth); if (!IsConfirmedBitcoinBlock(block_hash, required_depth, num_txs)) { err_msg = "Needs more confirmations."; if (depth_failed) { From 24cafa50cfcc44bdf015503c70d877f38fa7978c Mon Sep 17 00:00:00 2001 From: Pablo Greco Date: Thu, 28 Oct 2021 11:26:53 +0000 Subject: [PATCH 07/12] Implement vbparams for Elements (evbparams) --- src/chainparams.cpp | 97 +++++++++++++++++-- src/chainparamsbase.cpp | 2 +- .../feature_elements_taproot_activation.py | 4 +- 3 files changed, 93 insertions(+), 10 deletions(-) diff --git a/src/chainparams.cpp b/src/chainparams.cpp index accd3727fd..5935493ecc 100644 --- a/src/chainparams.cpp +++ b/src/chainparams.cpp @@ -85,6 +85,89 @@ static CBlock CreateGenesisBlock(uint32_t nTime, uint32_t nNonce, uint32_t nBits return CreateGenesisBlock(params, genesisScriptSig, genesisOutputScript, nTime, nNonce, nBits, nVersion, genesisReward); } +/** + * Allows modifying the Version Bits Elements regtest parameters (and liquidv1test). + * Ideally, this would be a method in the base class, inherited everywhere, but that might complicate future merges, + * so we settle for this static function. + */ +static void UpdateElementsActivationParametersFromArgs(Consensus::Params& consensus, const ArgsManager& args) +{ + if (!args.IsArgSet("-evbparams")) return; + + for (const std::string& strDeployment : args.GetArgs("-evbparams")) { + std::vector vDeploymentParams; + boost::split(vDeploymentParams, strDeployment, boost::is_any_of(":")); + if (vDeploymentParams.size() != 5) { + throw std::runtime_error("ElementsVersion bits parameters malformed, expecting deployment:start:end:period:threshold"); + } + int64_t nStartTime = 0, nTimeout = 0, nPeriod = 0, nThreshold = 0; + bool use_nStartTime = false, use_nTimeout = false, use_nPeriod = false, use_nThreshold = false; + if(vDeploymentParams[1].length()) { + if (!ParseInt64(vDeploymentParams[1], &nStartTime)) { + throw std::runtime_error(strprintf("Invalid nStartTime (%s)", vDeploymentParams[1])); + } + use_nStartTime = true; + } + if(vDeploymentParams[2].length()) { + if (!ParseInt64(vDeploymentParams[2], &nTimeout)) { + throw std::runtime_error(strprintf("Invalid nTimeout (%s)", vDeploymentParams[2])); + } + use_nTimeout = true; + } + if(vDeploymentParams[3].length()) { + if (!ParseInt64(vDeploymentParams[3], &nPeriod)) { + throw std::runtime_error(strprintf("Invalid nPeriod (%s)", vDeploymentParams[3])); + } + use_nPeriod = true; + } + if(vDeploymentParams[4].length()) { + if (!ParseInt64(vDeploymentParams[4], &nThreshold)) { + throw std::runtime_error(strprintf("Invalid nThreshold (%s)", vDeploymentParams[4])); + } + use_nThreshold = true; + } + bool found = false; + for (int j=0; j < (int)Consensus::MAX_VERSION_BITS_DEPLOYMENTS; ++j) { + if (vDeploymentParams[0] == VersionBitsDeploymentInfo[j].name) { + std::string extra_logging; + Consensus::DeploymentPos d=Consensus::DeploymentPos(j); + if (use_nStartTime) { + consensus.vDeployments[d].nStartTime = nStartTime; + } else { + nStartTime =consensus.vDeployments[d].nStartTime; + } + if (use_nTimeout) { + consensus.vDeployments[d].nTimeout = nTimeout; + } else { + nTimeout = consensus.vDeployments[d].nTimeout; + } + if (consensus.vDeployments[d].nPeriod) { + if(use_nPeriod) { + consensus.vDeployments[d].nPeriod = nPeriod; + } else { + nPeriod = *consensus.vDeployments[d].nPeriod; + } + extra_logging+= strprintf(", period=%ld", nPeriod); + } + if (consensus.vDeployments[d].nThreshold) { + if(use_nThreshold) { + consensus.vDeployments[d].nThreshold = nThreshold; + } else { + nThreshold = *consensus.vDeployments[d].nThreshold; + } + extra_logging+= strprintf(", threshold=%ld", nThreshold); + } + found = true; + LogPrintf("Setting version bits activation parameters for %s to start=%ld, timeout=%ld%s\n", vDeploymentParams[0], nStartTime, nTimeout, extra_logging.c_str()); + break; + } + } + if (!found) { + throw std::runtime_error(strprintf("Invalid deployment (%s)", vDeploymentParams[0])); + } + } +} + /** * Main network */ @@ -478,7 +561,7 @@ public: consensus.vDeployments[Consensus::DEPLOYMENT_DYNA_FED].nStartTime = 1199145601; // January 1, 2008 consensus.vDeployments[Consensus::DEPLOYMENT_DYNA_FED].nTimeout = 1230767999; // December 31, 2008 consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].bit = 2; - consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].nStartTime = gArgs.GetArg("-con_taproot_signal_start", Consensus::BIP9Deployment::ALWAYS_ACTIVE); + consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].nStartTime = Consensus::BIP9Deployment::ALWAYS_ACTIVE; consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].nTimeout = Consensus::BIP9Deployment::NO_TIMEOUT; consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].nPeriod = 128; // test ability to change from default consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].nThreshold = 128; @@ -751,6 +834,8 @@ class CCustomParams : public CRegTestParams { consensus.vDeployments[Consensus::DEPLOYMENT_DYNA_FED].bit = 25; consensus.vDeployments[Consensus::DEPLOYMENT_DYNA_FED].nStartTime = args.GetArg("-con_dyna_deploy_start", Consensus::BIP9Deployment::ALWAYS_ACTIVE); consensus.vDeployments[Consensus::DEPLOYMENT_DYNA_FED].nTimeout = Consensus::BIP9Deployment::NO_TIMEOUT; + + UpdateElementsActivationParametersFromArgs(consensus, args); // END ELEMENTS fields } @@ -1064,6 +1149,9 @@ public: // For testing purposes, default to the same junk keys that CustomParams uses (this can be overridden.) consensus.first_extension_space = {ParseHex("02fcba7ecf41bc7e1be4ee122d9d22e3333671eb0a3a87b5cdf099d59874e1940f02fcba7ecf41bc7e1be4ee122d9d22e3333671eb0a3a87b5cdf099d59874e1940f")}; + // Don't use liquidv1's height to enable taproot + consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].nStartTime = 0; + // Use all regtest rather than mainchain magic numbers: bech32_hrp = args.GetArg("-bech32_hrp", "ert"); blech32_hrp = args.GetArg("-blech32_hrp", "el"); @@ -1101,9 +1189,6 @@ public: // This is unlike the CCustomParams UpdateFromArgs method, which has lots of defaults in it. void UpdateFromArgs(const ArgsManager& args) { - // NOTE: We don't handle version bits, because I'm not sure we actually use them, and it would be messy to do so. - // UpdateVersionBitsParametersFromArgs(args); - consensus.nSubsidyHalvingInterval = args.GetArg("-con_nsubsidyhalvinginterval", consensus.nSubsidyHalvingInterval); if (args.IsArgSet("-con_bip16exception")) { consensus.BIP16Exception = uint256S(args.GetArg("-con_bip16exception", "")); @@ -1275,9 +1360,7 @@ public: consensus.vDeployments[Consensus::DEPLOYMENT_DYNA_FED].nTimeout = Consensus::BIP9Deployment::NO_TIMEOUT; } - if (args.IsArgSet("-con_taproot_signal_start")) { - consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].nStartTime = gArgs.GetArg("-con_taproot_signal_start", 0); - } + UpdateElementsActivationParametersFromArgs(consensus, args); // END ELEMENTS fields } diff --git a/src/chainparamsbase.cpp b/src/chainparamsbase.cpp index a37b5742a9..7ff36cfc76 100644 --- a/src/chainparamsbase.cpp +++ b/src/chainparamsbase.cpp @@ -60,7 +60,7 @@ void SetupChainParamsBaseOptions(ArgsManager& argsman) argsman.AddArg("-con_dyna_deploy_signal", "Whether to signal for the Dynamic Federations deployment (default: false).", ArgsManager::ALLOW_ANY, OptionsCategory::ELEMENTS); argsman.AddArg("-dynamic_epoch_length", "Per-chain parameter that sets how many blocks dynamic federation voting and enforcement are in effect for.", ArgsManager::ALLOW_ANY, OptionsCategory::ELEMENTS); argsman.AddArg("-total_valid_epochs", "Per-chain parameter that sets how long a particular fedpegscript is in effect for.", ArgsManager::ALLOW_ANY, OptionsCategory::ELEMENTS); - argsman.AddArg("-con_taproot_signal_start", "Whether, and at what blockheight, to start signalling for Taproot activation (default: false) (regtest, Liquid testnet, or custom only).", ArgsManager::ALLOW_ANY, OptionsCategory::ELEMENTS); + argsman.AddArg("-evbparams=deployment:start:end:period:threshold", "Use given start/end times for specified version bits deployment (regtest or custom only)", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::ELEMENTS); // END ELEMENTS // } diff --git a/test/functional/feature_elements_taproot_activation.py b/test/functional/feature_elements_taproot_activation.py index 5a476381e2..2fc477dff5 100755 --- a/test/functional/feature_elements_taproot_activation.py +++ b/test/functional/feature_elements_taproot_activation.py @@ -98,7 +98,7 @@ class TaprootActivationTest(BitcoinTestFramework): assert_equal(rpc.getblockheader(blocks[0])["versionHex"], "20000000") def run_test(self): - # Test that regtest nodes without -con_taproot_signal_start never signal + # Test that regtest nodes never signal taproot by default self.log.info("Testing node not configured to activate taproot") blocks = self.nodes[0].generatetoaddress(2500, self.nodes[0].getnewaddress()) assert_equal(self.nodes[0].getblockcount(), 2500) @@ -113,7 +113,7 @@ class TaprootActivationTest(BitcoinTestFramework): assert_equal (decode["versionHex"], "20000000") # Test activation starting from height 1000 - self.restart_node(0, ["-con_taproot_signal_start=500"]) + self.restart_node(0, ["-evbparams=taproot:500:::"]) self.nodes[0].invalidateblock(self.nodes[0].getblockhash(1)) self.test_activation(self.nodes[0], 500) From 19f54eb5e0f5b3d4604c4808435d2d2e4a4772f2 Mon Sep 17 00:00:00 2001 From: Pablo Greco Date: Sat, 30 Oct 2021 09:35:07 +0000 Subject: [PATCH 08/12] Make sure command line takes precedence when processing evbparams --- src/chainparams.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/chainparams.cpp b/src/chainparams.cpp index 5935493ecc..1fb1066299 100644 --- a/src/chainparams.cpp +++ b/src/chainparams.cpp @@ -94,6 +94,7 @@ static void UpdateElementsActivationParametersFromArgs(Consensus::Params& consen { if (!args.IsArgSet("-evbparams")) return; + std::map map_deployments; for (const std::string& strDeployment : args.GetArgs("-evbparams")) { std::vector vDeploymentParams; boost::split(vDeploymentParams, strDeployment, boost::is_any_of(":")); @@ -129,7 +130,13 @@ static void UpdateElementsActivationParametersFromArgs(Consensus::Params& consen bool found = false; for (int j=0; j < (int)Consensus::MAX_VERSION_BITS_DEPLOYMENTS; ++j) { if (vDeploymentParams[0] == VersionBitsDeploymentInfo[j].name) { + if(map_deployments[vDeploymentParams[0]]) { + found = true; + LogPrintf("Ignoring duplicated version bits activation parameters for \"%s\"\n", strDeployment.c_str()); + break; + } std::string extra_logging; + map_deployments[vDeploymentParams[0]]=1; Consensus::DeploymentPos d=Consensus::DeploymentPos(j); if (use_nStartTime) { consensus.vDeployments[d].nStartTime = nStartTime; From a06aac152a0b3535890c6a1f0118b255838ae6c5 Mon Sep 17 00:00:00 2001 From: Pablo Greco Date: Fri, 29 Oct 2021 16:59:21 +0000 Subject: [PATCH 09/12] Make dynafed signaling opt-out instead of opt-in --- src/chainparamsbase.cpp | 2 +- src/validation.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/chainparamsbase.cpp b/src/chainparamsbase.cpp index 7ff36cfc76..c87dd674e6 100644 --- a/src/chainparamsbase.cpp +++ b/src/chainparamsbase.cpp @@ -57,7 +57,7 @@ void SetupChainParamsBaseOptions(ArgsManager& argsman) argsman.AddArg("-multi_data_permitted", "Allow relay of multiple OP_RETURN outputs. (default: -enforce_pak)", ArgsManager::ALLOW_ANY, OptionsCategory::ELEMENTS); argsman.AddArg("-con_csv_deploy_start", "Starting height for CSV deployment. (default: -1, which means ACTIVE from genesis)", ArgsManager::ALLOW_ANY, OptionsCategory::ELEMENTS); argsman.AddArg("-con_dyna_deploy_start", "Starting height for Dynamic Federations deployment. Once active, signblockscript becomes a BIP141 WSH scriptPubKey of the original signblockscript. All other dynamic parameters stay constant.(default: -1, which means ACTIVE from genesis)", ArgsManager::ALLOW_ANY, OptionsCategory::ELEMENTS); - argsman.AddArg("-con_dyna_deploy_signal", "Whether to signal for the Dynamic Federations deployment (default: false).", ArgsManager::ALLOW_ANY, OptionsCategory::ELEMENTS); + argsman.AddArg("-con_dyna_deploy_signal", "Whether to signal for the Dynamic Federations deployment (default: true).", ArgsManager::ALLOW_ANY, OptionsCategory::ELEMENTS); argsman.AddArg("-dynamic_epoch_length", "Per-chain parameter that sets how many blocks dynamic federation voting and enforcement are in effect for.", ArgsManager::ALLOW_ANY, OptionsCategory::ELEMENTS); argsman.AddArg("-total_valid_epochs", "Per-chain parameter that sets how long a particular fedpegscript is in effect for.", ArgsManager::ALLOW_ANY, OptionsCategory::ELEMENTS); argsman.AddArg("-evbparams=deployment:start:end:period:threshold", "Use given start/end times for specified version bits deployment (regtest or custom only)", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::ELEMENTS); diff --git a/src/validation.cpp b/src/validation.cpp index 24605b7bb6..cf50425145 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1982,7 +1982,7 @@ int32_t ComputeBlockVersion(const CBlockIndex* pindexPrev, const Consensus::Para } // Undo default signalling behavior for dynafed unless explicitly enabled. - if (!gArgs.GetBoolArg("-con_dyna_deploy_signal", false)) { + if (!gArgs.GetBoolArg("-con_dyna_deploy_signal", true)) { auto dynafed = Consensus::DeploymentPos::DEPLOYMENT_DYNA_FED; int bit = params.vDeployments[dynafed].bit; if (bit > 0 && bit < VERSIONBITS_NUM_BITS) { From d15d08c6055227ae5544b20cb7867231c702258c Mon Sep 17 00:00:00 2001 From: Pablo Greco Date: Fri, 29 Oct 2021 17:02:41 +0000 Subject: [PATCH 10/12] Dynafed is already activated in liquidv1, remove start argument parsing --- src/chainparams.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/chainparams.cpp b/src/chainparams.cpp index 1fb1066299..1e1de1ced8 100644 --- a/src/chainparams.cpp +++ b/src/chainparams.cpp @@ -1103,7 +1103,7 @@ public: // Activated from block 1,000,000. consensus.vDeployments[Consensus::DEPLOYMENT_DYNA_FED].bit = 25; // Allow blocksigners to delay activation. - consensus.vDeployments[Consensus::DEPLOYMENT_DYNA_FED].nStartTime = gArgs.GetArg("-con_dyna_deploy_start", 1000000); + consensus.vDeployments[Consensus::DEPLOYMENT_DYNA_FED].nStartTime = 1000000; consensus.vDeployments[Consensus::DEPLOYMENT_DYNA_FED].nTimeout = Consensus::BIP9Deployment::NO_TIMEOUT; From 85c3c2b819f06296487678ff431678a2d9645a1d Mon Sep 17 00:00:00 2001 From: Pablo Greco Date: Fri, 29 Oct 2021 17:23:07 +0000 Subject: [PATCH 11/12] Deprecate con_dyna_deploy_start in favor of evbparams --- contrib/assets_tutorial/assets_tutorial.py | 2 +- src/chainparams.cpp | 10 ++++------ src/chainparamsbase.cpp | 1 - .../functional/test_framework/util.py | 2 +- test/functional/feature_blocksign.py | 2 +- test/functional/feature_dynafed.py | 4 ++-- test/functional/feature_fedpeg.py | 2 +- test/functional/feature_pak.py | 2 +- test/functional/feature_sighash_rangeproof.py | 2 +- test/functional/rpc_tweakfedpeg.py | 2 +- test/functional/test_framework/util.py | 2 +- 11 files changed, 14 insertions(+), 17 deletions(-) diff --git a/contrib/assets_tutorial/assets_tutorial.py b/contrib/assets_tutorial/assets_tutorial.py index 8e0919768a..3b933a17a0 100755 --- a/contrib/assets_tutorial/assets_tutorial.py +++ b/contrib/assets_tutorial/assets_tutorial.py @@ -482,7 +482,7 @@ extra_args = [ "-con_max_block_sig_size=150", # We also disable dynamic federations, since we are not going to do any # dynafed transitions in this tutorial. FIXME we probably should. - "-con_dyna_deploy_start=0", + "-evbparams=dynafed:0:::", ] print ("6b. Restart both nodes") diff --git a/src/chainparams.cpp b/src/chainparams.cpp index 1e1de1ced8..f0086a6864 100644 --- a/src/chainparams.cpp +++ b/src/chainparams.cpp @@ -839,7 +839,7 @@ class CCustomParams : public CRegTestParams { } consensus.vDeployments[Consensus::DEPLOYMENT_DYNA_FED].bit = 25; - consensus.vDeployments[Consensus::DEPLOYMENT_DYNA_FED].nStartTime = args.GetArg("-con_dyna_deploy_start", Consensus::BIP9Deployment::ALWAYS_ACTIVE); + consensus.vDeployments[Consensus::DEPLOYMENT_DYNA_FED].nStartTime = Consensus::BIP9Deployment::ALWAYS_ACTIVE; consensus.vDeployments[Consensus::DEPLOYMENT_DYNA_FED].nTimeout = Consensus::BIP9Deployment::NO_TIMEOUT; UpdateElementsActivationParametersFromArgs(consensus, args); @@ -1361,11 +1361,9 @@ public: consensus.subsidy_asset = CAsset(uint256S(args.GetArg("-subsidyasset", ""))); } - if (args.IsArgSet("-con_dyna_deploy_start")) { - consensus.vDeployments[Consensus::DEPLOYMENT_DYNA_FED].bit = 25; - consensus.vDeployments[Consensus::DEPLOYMENT_DYNA_FED].nStartTime = args.GetArg("-con_dyna_deploy_start", Consensus::BIP9Deployment::ALWAYS_ACTIVE); - consensus.vDeployments[Consensus::DEPLOYMENT_DYNA_FED].nTimeout = Consensus::BIP9Deployment::NO_TIMEOUT; - } + consensus.vDeployments[Consensus::DEPLOYMENT_DYNA_FED].bit = 25; + consensus.vDeployments[Consensus::DEPLOYMENT_DYNA_FED].nStartTime = Consensus::BIP9Deployment::ALWAYS_ACTIVE; + consensus.vDeployments[Consensus::DEPLOYMENT_DYNA_FED].nTimeout = Consensus::BIP9Deployment::NO_TIMEOUT; UpdateElementsActivationParametersFromArgs(consensus, args); diff --git a/src/chainparamsbase.cpp b/src/chainparamsbase.cpp index c87dd674e6..7173ccf69b 100644 --- a/src/chainparamsbase.cpp +++ b/src/chainparamsbase.cpp @@ -56,7 +56,6 @@ void SetupChainParamsBaseOptions(ArgsManager& argsman) argsman.AddArg("-pak", "Sets the 'first extension space' field to the pak entries ala pre-dynamic federations. Only used for testing in custom chains.", ArgsManager::ALLOW_ANY, OptionsCategory::ELEMENTS); argsman.AddArg("-multi_data_permitted", "Allow relay of multiple OP_RETURN outputs. (default: -enforce_pak)", ArgsManager::ALLOW_ANY, OptionsCategory::ELEMENTS); argsman.AddArg("-con_csv_deploy_start", "Starting height for CSV deployment. (default: -1, which means ACTIVE from genesis)", ArgsManager::ALLOW_ANY, OptionsCategory::ELEMENTS); - argsman.AddArg("-con_dyna_deploy_start", "Starting height for Dynamic Federations deployment. Once active, signblockscript becomes a BIP141 WSH scriptPubKey of the original signblockscript. All other dynamic parameters stay constant.(default: -1, which means ACTIVE from genesis)", ArgsManager::ALLOW_ANY, OptionsCategory::ELEMENTS); argsman.AddArg("-con_dyna_deploy_signal", "Whether to signal for the Dynamic Federations deployment (default: true).", ArgsManager::ALLOW_ANY, OptionsCategory::ELEMENTS); argsman.AddArg("-dynamic_epoch_length", "Per-chain parameter that sets how many blocks dynamic federation voting and enforcement are in effect for.", ArgsManager::ALLOW_ANY, OptionsCategory::ELEMENTS); argsman.AddArg("-total_valid_epochs", "Per-chain parameter that sets how long a particular fedpegscript is in effect for.", ArgsManager::ALLOW_ANY, OptionsCategory::ELEMENTS); diff --git a/test/bitcoin_functional/functional/test_framework/util.py b/test/bitcoin_functional/functional/test_framework/util.py index 303f501488..cb83ec20de 100644 --- a/test/bitcoin_functional/functional/test_framework/util.py +++ b/test/bitcoin_functional/functional/test_framework/util.py @@ -322,7 +322,7 @@ def initialize_datadir(dirname, n): f.write("pubkeyprefix=111\n") f.write("scriptprefix=196\n") f.write("bech32_hrp=bcrt\n") - f.write("con_dyna_deploy_start="+str(2**31)+"\n") # Never starts + f.write("evbparams=dynafed:"+str(2**31)+":::\n") # Never starts f.write("minrelaytxfee=0.00001\n") os.makedirs(os.path.join(datadir, 'stderr'), exist_ok=True) os.makedirs(os.path.join(datadir, 'stdout'), exist_ok=True) diff --git a/test/functional/feature_blocksign.py b/test/functional/feature_blocksign.py index 363664afe2..b8c55af5b6 100755 --- a/test/functional/feature_blocksign.py +++ b/test/functional/feature_blocksign.py @@ -82,7 +82,7 @@ class BlockSignTest(BitcoinTestFramework): "-signblockscript={}".format(signblockscript), "-con_max_block_sig_size={}".format(self.required_signers*74+self.num_nodes*33), "-anyonecanspendaremine=1", - "-con_dyna_deploy_start=0", + "-evbparams=dynafed:0:::", "-con_dyna_deploy_signal=1", ]] * self.num_nodes diff --git a/test/functional/feature_dynafed.py b/test/functional/feature_dynafed.py index 1e219fc521..32322602ad 100755 --- a/test/functional/feature_dynafed.py +++ b/test/functional/feature_dynafed.py @@ -67,7 +67,7 @@ class DynaFedTest(BitcoinTestFramework): self.num_nodes = 2 # We want to test activation of dynafed self.extra_args = [[ - "-con_dyna_deploy_start=1000", + "-evbparams=dynafed:1000:::", "-enforce_pak=1", "-con_parent_chain_signblockscript=51", "-peginconfirmationdepth=1", @@ -78,7 +78,7 @@ class DynaFedTest(BitcoinTestFramework): # second node will not mine transactions self.extra_args[1].append("-blocksonly=1") # Make sure nothing breaks if peers have a different activation. - self.extra_args[1][0] = "-con_dyna_deploy_start=937" + self.extra_args[1][0] = "-evbparams=dynafed:937:::" def skip_test_if_missing_module(self): self.skip_if_no_wallet() diff --git a/test/functional/feature_fedpeg.py b/test/functional/feature_fedpeg.py index b727708b9c..7ad5e137df 100755 --- a/test/functional/feature_fedpeg.py +++ b/test/functional/feature_fedpeg.py @@ -140,7 +140,7 @@ class FedPegTest(BitcoinTestFramework): # Immediate activation of dynafed when requested versus "never" from conf if self.options.pre_transition or self.options.post_transition: - extra_args.extend(["-con_dyna_deploy_start=-1"]) + extra_args.extend(["-evbparams=dynafed:-1:::"]) # Use rpcuser auth only for first parent. if n==0: diff --git a/test/functional/feature_pak.py b/test/functional/feature_pak.py index 7fd8479f67..5eb3598bb5 100755 --- a/test/functional/feature_pak.py +++ b/test/functional/feature_pak.py @@ -18,7 +18,7 @@ class PAKTest (BitcoinTestFramework): def set_test_params(self): self.num_nodes = 3 self.setup_clean_chain = True - self.extra_args = [["-enforce_pak=1", "-con_dyna_deploy_start=-1", "-initialfreecoins=210000000000000", "-anyonecanspendaremine=1", "-parent_bech32_hrp=lol", "-pubkeyprefix=112", "-scriptprefix=197", "-con_connect_genesis_outputs=1"] for i in range(self.num_nodes)] + self.extra_args = [["-enforce_pak=1", "-evbparams=dynafed:-1:::", "-initialfreecoins=210000000000000", "-anyonecanspendaremine=1", "-parent_bech32_hrp=lol", "-pubkeyprefix=112", "-scriptprefix=197", "-con_connect_genesis_outputs=1"] for i in range(self.num_nodes)] # First node doesn't enforce PAK, a "HF" of the other two nodes self.extra_args[0] = ["-acceptnonstdtxn=1"] + self.extra_args[0][1:] ## FIXME -acceptnonstdtxn=1 should not be needed diff --git a/test/functional/feature_sighash_rangeproof.py b/test/functional/feature_sighash_rangeproof.py index 4ffdcf9ffc..0fb4dd7836 100755 --- a/test/functional/feature_sighash_rangeproof.py +++ b/test/functional/feature_sighash_rangeproof.py @@ -51,7 +51,7 @@ class SighashRangeproofTest(BitcoinTestFramework): self.num_nodes = 3 # We want to test activation of dynafed self.extra_args = [[ - "-con_dyna_deploy_start=1000", + "-evbparams=dynafed:1000:::", "-con_dyna_deploy_signal=1", "-blindedaddresses=1", "-initialfreecoins=2100000000000000", diff --git a/test/functional/rpc_tweakfedpeg.py b/test/functional/rpc_tweakfedpeg.py index 5f67b625ab..51685f27e2 100755 --- a/test/functional/rpc_tweakfedpeg.py +++ b/test/functional/rpc_tweakfedpeg.py @@ -26,7 +26,7 @@ class TweakFedpegTest(BitcoinTestFramework): [ "-fedpegscript="+LIQUID_SCRIPT, "-con_dyna_deploy_signal=1", - "-con_dyna_deploy_start=0", # test dynafed derivation + "-evbparams=dynafed:0:::", # test dynafed derivation ]] def setup_network(self): diff --git a/test/functional/test_framework/util.py b/test/functional/test_framework/util.py index 7abb757a6d..01a677cf34 100644 --- a/test/functional/test_framework/util.py +++ b/test/functional/test_framework/util.py @@ -381,7 +381,7 @@ def initialize_datadir(dirname, n, chain): f.write("con_bip65height=1351\n") f.write("con_bip66height=1251\n") f.write("blindedaddresses=0\n") # Set to minimize broken tests in favor of custom - f.write("con_dyna_deploy_start="+str(2**31)+"\n") # Never starts unless overridden + f.write("evbparams=dynafed:"+str(2**31)+":::\n") # Never starts unless overridden f.write("minrelaytxfee=0.00001\n") #f.write("pubkeyprefix=111\n") #f.write("scriptprefix=196\n") From a58b3b7ab3f51f38ba25d0648e41e010a228dd14 Mon Sep 17 00:00:00 2001 From: Pablo Greco Date: Fri, 26 Nov 2021 12:40:40 +0000 Subject: [PATCH 12/12] Remove obsolete comment after a06aac152a0b3535890c6a1f0118b255838ae6c5 --- src/validation.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/validation.cpp b/src/validation.cpp index a6819098e9..aa0aa7430e 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1981,7 +1981,6 @@ int32_t ComputeBlockVersion(const CBlockIndex* pindexPrev, const Consensus::Para } } - // Undo default signalling behavior for dynafed unless explicitly enabled. if (!gArgs.GetBoolArg("-con_dyna_deploy_signal", true)) { auto dynafed = Consensus::DeploymentPos::DEPLOYMENT_DYNA_FED; int bit = params.vDeployments[dynafed].bit;