From 31ca8a497a1088f65a2b604c720c055bdffaa289 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Mon, 28 Oct 2024 12:15:15 +0700 Subject: [PATCH 1/7] feat: update limit of withdrawals to flat 2000 starting from v22 --- src/chainparams.cpp | 6 +++--- src/evo/creditpool.cpp | 14 +++++++++----- src/evo/creditpool.h | 1 + test/functional/feature_asset_locks.py | 25 ++++++++++++++++++++++--- 4 files changed, 35 insertions(+), 11 deletions(-) diff --git a/src/chainparams.cpp b/src/chainparams.cpp index f910fc6021..a354f8ec18 100644 --- a/src/chainparams.cpp +++ b/src/chainparams.cpp @@ -812,9 +812,9 @@ public: consensus.vDeployments[Consensus::DEPLOYMENT_WITHDRAWALS].bit = 11; consensus.vDeployments[Consensus::DEPLOYMENT_WITHDRAWALS].nStartTime = 0; consensus.vDeployments[Consensus::DEPLOYMENT_WITHDRAWALS].nTimeout = Consensus::BIP9Deployment::NO_TIMEOUT; - consensus.vDeployments[Consensus::DEPLOYMENT_WITHDRAWALS].nWindowSize = 300; - consensus.vDeployments[Consensus::DEPLOYMENT_WITHDRAWALS].nThresholdStart = 300 / 5 * 4; // 80% of 12 - consensus.vDeployments[Consensus::DEPLOYMENT_WITHDRAWALS].nThresholdMin = 300 / 5 * 3; // 60% of 7 + consensus.vDeployments[Consensus::DEPLOYMENT_WITHDRAWALS].nWindowSize = 900; + consensus.vDeployments[Consensus::DEPLOYMENT_WITHDRAWALS].nThresholdStart = 900 / 5 * 4; // 80% of window size + consensus.vDeployments[Consensus::DEPLOYMENT_WITHDRAWALS].nThresholdMin = 900 / 5 * 3; // 60% of window size consensus.vDeployments[Consensus::DEPLOYMENT_WITHDRAWALS].nFalloffCoeff = 5; // this corresponds to 10 periods consensus.vDeployments[Consensus::DEPLOYMENT_WITHDRAWALS].useEHF = true; diff --git a/src/evo/creditpool.cpp b/src/evo/creditpool.cpp index 374f45792c..cfbea96e5d 100644 --- a/src/evo/creditpool.cpp +++ b/src/evo/creditpool.cpp @@ -171,14 +171,18 @@ CCreditPool CCreditPoolManager::ConstructCreditPool(const CBlockIndex* const blo } } - // Unlock limits are # max(100, min(.10 * assetlockpool, 1000)) inside window CAmount currentLimit = locked; const CAmount latelyUnlocked = prev.latelyUnlocked + blockData.unlocked - distantUnlocked; - if (currentLimit + latelyUnlocked > LimitAmountLow) { - currentLimit = std::max(LimitAmountLow, locked / 10) - latelyUnlocked; - if (currentLimit < 0) currentLimit = 0; + if (DeploymentActiveAt(*block_index, Params().GetConsensus(), Consensus::DEPLOYMENT_WITHDRAWALS)) { + currentLimit = std::min(currentLimit, LimitAmountV22); + } else { + // Unlock limits in pre-v22 are max(100, min(.10 * assetlockpool, 1000)) inside window + if (currentLimit + latelyUnlocked > LimitAmountLow) { + currentLimit = std::max(LimitAmountLow, locked / 10) - latelyUnlocked; + if (currentLimit < 0) currentLimit = 0; + } + currentLimit = std::min(currentLimit, LimitAmountHigh - latelyUnlocked); } - currentLimit = std::min(currentLimit, LimitAmountHigh - latelyUnlocked); assert(currentLimit >= 0); diff --git a/src/evo/creditpool.h b/src/evo/creditpool.h index 14df187bfc..9545922caf 100644 --- a/src/evo/creditpool.h +++ b/src/evo/creditpool.h @@ -117,6 +117,7 @@ public: static constexpr int LimitBlocksToTrace = 576; static constexpr CAmount LimitAmountLow = 100 * COIN; static constexpr CAmount LimitAmountHigh = 1000 * COIN; + static constexpr CAmount LimitAmountV22 = 2000 * COIN; explicit CCreditPoolManager(CEvoDB& _evoDb); diff --git a/test/functional/feature_asset_locks.py b/test/functional/feature_asset_locks.py index ff6ac61289..c59a3f0e5d 100755 --- a/test/functional/feature_asset_locks.py +++ b/test/functional/feature_asset_locks.py @@ -273,7 +273,7 @@ class AssetLocksTest(DashTestFramework): self.test_asset_unlocks(node_wallet, node, pubkey) self.test_withdrawal_limits(node_wallet, node, pubkey) self.test_mn_rr(node_wallet, node, pubkey) - self.test_withdrawal_fork(node_wallet, pubkey) + self.test_withdrawal_fork(node_wallet, node, pubkey) def test_asset_locks(self, node_wallet, node, pubkey): @@ -466,7 +466,9 @@ class AssetLocksTest(DashTestFramework): def test_withdrawal_limits(self, node_wallet, node, pubkey): - self.log.info("Testing withdrawal limits...") + self.log.info("Testing withdrawal limits before v22 'withdrawal fork'...") + assert not softfork_active(node_wallet, 'withdrawals') + self.log.info("Too big withdrawal is expected to not be mined") asset_unlock_tx_full = self.create_assetunlock(201, 1 + self.get_credit_pool_balance(), pubkey) @@ -615,6 +617,7 @@ class AssetLocksTest(DashTestFramework): self.log.info("Checking that credit pool is not changed...") assert_equal(new_total, self.get_credit_pool_balance()) self.check_mempool_size() + assert not softfork_active(node_wallet, 'withdrawals') def test_mn_rr(self, node_wallet, node, pubkey): @@ -645,8 +648,9 @@ class AssetLocksTest(DashTestFramework): self.generate(node, 1) assert_equal(locked, self.get_credit_pool_balance()) - def test_withdrawal_fork(self, node_wallet, pubkey): + def test_withdrawal_fork(self, node_wallet, node, pubkey): self.log.info("Testing asset unlock after 'withdrawal' activation...") + self.activate_by_name('withdrawals') assert softfork_active(node_wallet, 'withdrawals') index = 501 @@ -684,6 +688,21 @@ class AssetLocksTest(DashTestFramework): self.mine_quorum_2_nodes(llmq_type_name="llmq_test_platform", llmq_type=106) self.check_mempool_result(tx=asset_unlock_tx, result_expected={'allowed': False, 'reject-reason': 'bad-assetunlock-too-old-quorum'}) + asset_unlock_tx = self.create_assetunlock(520, 2000 * COIN + 1, pubkey) + txid_in_block = self.send_tx(asset_unlock_tx) + self.generate(node, 1) + self.ensure_tx_is_not_mined(txid_in_block) + + asset_unlock_tx = self.create_assetunlock(521, 2000 * COIN, pubkey) + txid_in_block = self.send_tx(asset_unlock_tx) + self.generate(node, 1) + block = node.getblock(node.getbestblockhash()) + assert txid_in_block in block['tx'] + + asset_unlock_tx = self.create_assetunlock(522, COIN, pubkey) + txid_in_block = self.send_tx(asset_unlock_tx) + self.generate(node, 1) + self.ensure_tx_is_not_mined(txid_in_block) if __name__ == '__main__': AssetLocksTest().main() From 5b0a2f56cdd3021ad76cfaa0c3016a3cfd100587 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Mon, 28 Oct 2024 15:05:32 +0700 Subject: [PATCH 2/7] fix: string in credit pool logs 'previous' is renamed to recently unlocked --- src/evo/creditpool.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/evo/creditpool.cpp b/src/evo/creditpool.cpp index cfbea96e5d..66fb9a75fd 100644 --- a/src/evo/creditpool.cpp +++ b/src/evo/creditpool.cpp @@ -187,7 +187,7 @@ CCreditPool CCreditPoolManager::ConstructCreditPool(const CBlockIndex* const blo assert(currentLimit >= 0); if (currentLimit > 0 || latelyUnlocked > 0 || locked > 0) { - LogPrint(BCLog::CREDITPOOL, "CCreditPoolManager: asset unlock limits on height: %d locked: %d.%08d limit: %d.%08d previous: %d.%08d\n", + LogPrint(BCLog::CREDITPOOL, "CCreditPoolManager: asset unlock limits on height: %d locked: %d.%08d limit: %d.%08d unlocked-in-window: %d.%08d\n", block_index->nHeight, locked / COIN, locked % COIN, currentLimit / COIN, currentLimit % COIN, latelyUnlocked / COIN, latelyUnlocked % COIN); From ef6190e43471b3d518957f0e7f7fd008825c2d77 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Mon, 28 Oct 2024 15:27:30 +0700 Subject: [PATCH 3/7] docs: add release notes for withdrawal changes in v22 --- doc/release-notes-6279.md | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 doc/release-notes-6279.md diff --git a/doc/release-notes-6279.md b/doc/release-notes-6279.md new file mode 100644 index 0000000000..f7c73d3cf1 --- /dev/null +++ b/doc/release-notes-6279.md @@ -0,0 +1,8 @@ +# Notable changes + +## Asset Unlock transactions (platform transfer) + +This version introduces a new fork `withdrawals` that changes consensus rules. +New logic of validation of Asset Unlock transactions's signature. It let to use all 24 active quorums and the most recent inactive, while previous version of Dash Core may refuse withdrawal with error `bad-assetunlock-not-active-quorum` even if quorum is active. + +Limits for withdrawals has been increased to flat 2000 Dash per 576 latest blocks. From a51ade5cc93cdfbf0aca2b6cabca314d1a8fed2b Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Mon, 28 Oct 2024 15:54:52 +0700 Subject: [PATCH 4/7] style: apply clang-format --- src/evo/creditpool.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/evo/creditpool.cpp b/src/evo/creditpool.cpp index 66fb9a75fd..8f22254b29 100644 --- a/src/evo/creditpool.cpp +++ b/src/evo/creditpool.cpp @@ -187,10 +187,11 @@ CCreditPool CCreditPoolManager::ConstructCreditPool(const CBlockIndex* const blo assert(currentLimit >= 0); if (currentLimit > 0 || latelyUnlocked > 0 || locked > 0) { - LogPrint(BCLog::CREDITPOOL, "CCreditPoolManager: asset unlock limits on height: %d locked: %d.%08d limit: %d.%08d unlocked-in-window: %d.%08d\n", - block_index->nHeight, locked / COIN, locked % COIN, - currentLimit / COIN, currentLimit % COIN, - latelyUnlocked / COIN, latelyUnlocked % COIN); + LogPrint(BCLog::CREDITPOOL, /* Continued */ + "CCreditPoolManager: asset unlock limits on height: %d locked: %d.%08d limit: %d.%08d " + "unlocked-in-window: %d.%08d\n", + block_index->nHeight, locked / COIN, locked % COIN, currentLimit / COIN, currentLimit % COIN, + latelyUnlocked / COIN, latelyUnlocked % COIN); } CCreditPool pool{locked, currentLimit, latelyUnlocked, indexes}; From 877aa081445f00c929e0b82e102d9d7b5e7f9dfd Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Mon, 28 Oct 2024 20:56:03 +0700 Subject: [PATCH 5/7] feat: generate less blocks in feature_asset_locks.py to make it faster --- src/chainparams.cpp | 6 +++--- test/functional/feature_asset_locks.py | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/chainparams.cpp b/src/chainparams.cpp index a354f8ec18..c27d1cf5aa 100644 --- a/src/chainparams.cpp +++ b/src/chainparams.cpp @@ -812,9 +812,9 @@ public: consensus.vDeployments[Consensus::DEPLOYMENT_WITHDRAWALS].bit = 11; consensus.vDeployments[Consensus::DEPLOYMENT_WITHDRAWALS].nStartTime = 0; consensus.vDeployments[Consensus::DEPLOYMENT_WITHDRAWALS].nTimeout = Consensus::BIP9Deployment::NO_TIMEOUT; - consensus.vDeployments[Consensus::DEPLOYMENT_WITHDRAWALS].nWindowSize = 900; - consensus.vDeployments[Consensus::DEPLOYMENT_WITHDRAWALS].nThresholdStart = 900 / 5 * 4; // 80% of window size - consensus.vDeployments[Consensus::DEPLOYMENT_WITHDRAWALS].nThresholdMin = 900 / 5 * 3; // 60% of window size + consensus.vDeployments[Consensus::DEPLOYMENT_WITHDRAWALS].nWindowSize = 600; + consensus.vDeployments[Consensus::DEPLOYMENT_WITHDRAWALS].nThresholdStart = 600 / 5 * 4; // 80% of window size + consensus.vDeployments[Consensus::DEPLOYMENT_WITHDRAWALS].nThresholdMin = 600 / 5 * 3; // 60% of window size consensus.vDeployments[Consensus::DEPLOYMENT_WITHDRAWALS].nFalloffCoeff = 5; // this corresponds to 10 periods consensus.vDeployments[Consensus::DEPLOYMENT_WITHDRAWALS].useEHF = true; diff --git a/test/functional/feature_asset_locks.py b/test/functional/feature_asset_locks.py index c59a3f0e5d..1bc8309c6b 100755 --- a/test/functional/feature_asset_locks.py +++ b/test/functional/feature_asset_locks.py @@ -613,7 +613,7 @@ class AssetLocksTest(DashTestFramework): self.log.info("generate many blocks to be sure that mempool is empty after expiring txes...") - self.generate_batch(60) + self.generate_batch(HEIGHT_DIFF_EXPIRING) self.log.info("Checking that credit pool is not changed...") assert_equal(new_total, self.get_credit_pool_balance()) self.check_mempool_size() @@ -624,7 +624,7 @@ class AssetLocksTest(DashTestFramework): self.log.info("Activate mn_rr...") locked = self.get_credit_pool_balance() self.activate_mn_rr(expected_activation_height=2500) - self.log.info(f'height: {node.getblockcount()} credit: {self.get_credit_pool_balance()}') + self.log.info(f'mn-rr height: {node.getblockcount()} credit: {self.get_credit_pool_balance()}') assert_equal(locked, self.get_credit_pool_balance()) bt = node.getblocktemplate() @@ -650,8 +650,8 @@ class AssetLocksTest(DashTestFramework): def test_withdrawal_fork(self, node_wallet, node, pubkey): self.log.info("Testing asset unlock after 'withdrawal' activation...") - self.activate_by_name('withdrawals') assert softfork_active(node_wallet, 'withdrawals') + self.log.info(f'post-withdrawals height: {node.getblockcount()} credit: {self.get_credit_pool_balance()}') index = 501 while index < 511: From c97f5f5ca5e51cc0da6eae9a950ff96714c41740 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Mon, 28 Oct 2024 22:20:31 +0700 Subject: [PATCH 6/7] feat: update some asserts related to CreditPool in consensus code to exceptions --- src/evo/creditpool.cpp | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/evo/creditpool.cpp b/src/evo/creditpool.cpp index 8f22254b29..b3474a78a3 100644 --- a/src/evo/creditpool.cpp +++ b/src/evo/creditpool.cpp @@ -134,9 +134,13 @@ CCreditPool CCreditPoolManager::ConstructCreditPool(const CBlockIndex* const blo if (!block) { // If reading of previous block is not successfully, but // prev contains credit pool related data, something strange happened - assert(prev.locked == 0); - assert(prev.indexes.IsEmpty()); - + if (prev.locked != 0) { + throw std::runtime_error(strprintf("Failed to create CreditPool but previous block has value")); + } + if (!prev.indexes.IsEmpty()) { + throw std::runtime_error( + strprintf("Failed to create CreditPool but asset unlock transactions already mined")); + } CCreditPool emptyPool; AddToCache(block_index->GetBlockHash(), block_index->nHeight, emptyPool); return emptyPool; @@ -184,9 +188,7 @@ CCreditPool CCreditPoolManager::ConstructCreditPool(const CBlockIndex* const blo currentLimit = std::min(currentLimit, LimitAmountHigh - latelyUnlocked); } - assert(currentLimit >= 0); - - if (currentLimit > 0 || latelyUnlocked > 0 || locked > 0) { + if (currentLimit != 0 || latelyUnlocked > 0 || locked > 0) { LogPrint(BCLog::CREDITPOOL, /* Continued */ "CCreditPoolManager: asset unlock limits on height: %d locked: %d.%08d limit: %d.%08d " "unlocked-in-window: %d.%08d\n", @@ -194,6 +196,11 @@ CCreditPool CCreditPoolManager::ConstructCreditPool(const CBlockIndex* const blo latelyUnlocked / COIN, latelyUnlocked % COIN); } + if (currentLimit < 0) { + throw std::runtime_error( + strprintf("Negative limit for CreditPool: %d.%08d\n", currentLimit / COIN, currentLimit % COIN)); + } + CCreditPool pool{locked, currentLimit, latelyUnlocked, indexes}; AddToCache(block_index->GetBlockHash(), block_index->nHeight, pool); return pool; From e43ca6243a2dc5243ebffec680e6d0c7459d47d0 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Mon, 28 Oct 2024 22:25:11 +0700 Subject: [PATCH 7/7] feat: replace assert to error in p2p code of Asset Lock It is impossible situation which will never happen. But better to change it to exception for better error-prune implementation in case someone will decide to change this code: const auto quorums = qman.ScanQuorums(llmqType, pindexTip, quorums_to_scan); if (bool isActive = std::any_of(quorums.begin(), quorums.end(), [&](const auto &q) { return q->qc->quorumHash == quorumHash; }); !isActive) { return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-assetunlock-too-old-quorum"); } ... const auto quorum = qman.GetQuorum(llmqType, quorumHash); assert(quorum); <-- for sure exist because we just scanned quorums --- src/evo/assetlocktx.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/evo/assetlocktx.cpp b/src/evo/assetlocktx.cpp index 27e53e0b24..bd3a7e7581 100644 --- a/src/evo/assetlocktx.cpp +++ b/src/evo/assetlocktx.cpp @@ -138,7 +138,10 @@ bool CAssetUnlockPayload::VerifySig(const llmq::CQuorumManager& qman, const uint } const auto quorum = qman.GetQuorum(llmqType, quorumHash); - assert(quorum); + // quorum must be valid at this point. Let's check and throw error just in case + if (!quorum) { + return state.Invalid(TxValidationResult::TX_CONSENSUS, "internal-error"); + } const uint256 requestId = ::SerializeHash(std::make_pair(ASSETUNLOCK_REQUESTID_PREFIX, index));