Merge #6369: feat: increased withdrawal limits to flat 2000 from v22

e43ca6243a feat: replace assert to error in p2p code of Asset Lock (Konstantin Akimov)
c97f5f5ca5 feat: update some asserts related to CreditPool in consensus code to exceptions (Konstantin Akimov)
877aa08144 feat: generate less blocks in feature_asset_locks.py to make it faster (Konstantin Akimov)
a51ade5cc9 style: apply clang-format (Konstantin Akimov)
ef6190e434 docs: add release notes for withdrawal changes in v22 (Konstantin Akimov)
5b0a2f56cd fix: string in credit pool logs 'previous' is renamed to recently unlocked (Konstantin Akimov)
31ca8a497a feat: update limit of withdrawals to flat 2000 starting from v22 (Konstantin Akimov)

Pull request description:

  ## Issue being fixed or feature implemented
  Limit 1000 seems a bit small at the moment, while limit 2000 is still safe enough.

  ## What was done?
  Withdrawals limits in pre-v22 are:
   - if credit pool is more than 10k -> limit withdrawals to 1k
   - if credit pool is between 100 dash and 10000 dash -> let to withdraw 10%.
   - if 10% of credit pool is less than 100 dash -> no limits, let to withdraw everything.

  The fork `withdrawals` introduces higher limit:
   - 2000 dash per last 576 blocks. That's all.

  ## How Has This Been Tested?
  Updated functional test `feature_asset_locks.py`

  ## Breaking Changes
  Limits of withdrawals are increased to 2000 dash. It changes consensus rules.

  ## Checklist:
  - [x] I have performed a self-review of my own code
  - [x] I have commented my code, particularly in hard-to-understand areas
  - [x] I have added or updated relevant unit/integration/functional/e2e tests
  - [x] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone

ACKs for top commit:
  PastaPastaPasta:
    utACK e43ca6243a
  UdjinM6:
    utACK e43ca6243a

Tree-SHA512: 77fc27b6b38105cc311ee5ea78d66edfe854600ad6fb9422c0d302dac436e9aa1dcdc394a36ccb980d42de98091c1596e01be260b3a940df4e6309842fd89065
This commit is contained in:
pasta 2024-10-28 12:20:30 -05:00
commit 79ced62d6f
No known key found for this signature in database
GPG Key ID: E2F3D7916E722D38
6 changed files with 66 additions and 23 deletions

View File

@ -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.

View File

@ -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 = 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;

View File

@ -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));

View File

@ -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;
@ -171,22 +175,30 @@ 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);
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",
block_index->nHeight, locked / COIN, locked % COIN, currentLimit / COIN, currentLimit % COIN,
latelyUnlocked / COIN, latelyUnlocked % COIN);
}
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",
block_index->nHeight, locked / COIN, locked % COIN,
currentLimit / COIN, currentLimit % COIN,
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};

View File

@ -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);

View File

@ -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)
@ -611,17 +613,18 @@ 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()
assert not softfork_active(node_wallet, 'withdrawals')
def test_mn_rr(self, node_wallet, node, pubkey):
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()
@ -645,9 +648,10 @@ 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...")
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:
@ -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()