Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: new fork 'withdrawals' and fixes for asset unlock signature verification #6279

Merged
merged 6 commits into from
Oct 21, 2024
Merged

fix: new fork 'withdrawals' and fixes for asset unlock signature verification #6279

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0253438
feat: new fork WITHDRAWALS introduced
knst Sep 18, 2024
2704003
fix: using proper quorums for asset unlock validation
knst Sep 18, 2024
5fd7b07
chore: test new validation of asset unlocks tests after fork 'withdra…
knst Sep 18, 2024
6fbd128
chore: clang format/reword/typos
UdjinM6 Oct 11, 2024
ecd0a96
test: fix test_withdrawal_fork
UdjinM6 Oct 15, 2024
d690309
fix: limit amount of attempts for test `test_withdrawal_fork`
knst Oct 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions src/chainparams.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,15 @@ class CMainParams : public CChainParams {
consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].nTimeout = Consensus::BIP9Deployment::NO_TIMEOUT;
consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].min_activation_height = 0; // No activation delay

consensus.vDeployments[Consensus::DEPLOYMENT_WITHDRAWALS].bit = 11;
consensus.vDeployments[Consensus::DEPLOYMENT_WITHDRAWALS].nStartTime = 1728864000; // October 14, 2024
consensus.vDeployments[Consensus::DEPLOYMENT_WITHDRAWALS].nTimeout = 1760400000; // October 14, 2025
consensus.vDeployments[Consensus::DEPLOYMENT_WITHDRAWALS].nWindowSize = 4032;
consensus.vDeployments[Consensus::DEPLOYMENT_WITHDRAWALS].nThresholdStart = 3226; // 80% of 4032
consensus.vDeployments[Consensus::DEPLOYMENT_WITHDRAWALS].nThresholdMin = 2420; // 60% of 4032
consensus.vDeployments[Consensus::DEPLOYMENT_WITHDRAWALS].nFalloffCoeff = 5; // this corresponds to 10 periods
consensus.vDeployments[Consensus::DEPLOYMENT_WITHDRAWALS].useEHF = true;

// The best chain should have at least this much work.
consensus.nMinimumChainWork = uint256S("0x00000000000000000000000000000000000000000000988117deadb0db9cd5b8"); // 2109672

Expand Down Expand Up @@ -395,6 +404,15 @@ class CTestNetParams : public CChainParams {
consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].nTimeout = Consensus::BIP9Deployment::NO_TIMEOUT;
consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].min_activation_height = 0; // No activation delay

consensus.vDeployments[Consensus::DEPLOYMENT_WITHDRAWALS].bit = 11;
consensus.vDeployments[Consensus::DEPLOYMENT_WITHDRAWALS].nStartTime = 1728864000; // October 14, 2024
consensus.vDeployments[Consensus::DEPLOYMENT_WITHDRAWALS].nTimeout = Consensus::BIP9Deployment::NO_TIMEOUT;
consensus.vDeployments[Consensus::DEPLOYMENT_WITHDRAWALS].nWindowSize = 100;
consensus.vDeployments[Consensus::DEPLOYMENT_WITHDRAWALS].nThresholdStart = 80; // 80% of 100
consensus.vDeployments[Consensus::DEPLOYMENT_WITHDRAWALS].nThresholdMin = 60; // 60% of 100
consensus.vDeployments[Consensus::DEPLOYMENT_WITHDRAWALS].nFalloffCoeff = 5; // this corresponds to 10 periods
consensus.vDeployments[Consensus::DEPLOYMENT_WITHDRAWALS].useEHF = true;

// The best chain should have at least this much work.
consensus.nMinimumChainWork = uint256S("0x000000000000000000000000000000000000000000000000031779704a0f54b4"); // 1069875

Expand Down Expand Up @@ -556,6 +574,15 @@ class CDevNetParams : public CChainParams {
consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].nTimeout = Consensus::BIP9Deployment::NO_TIMEOUT;
consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].min_activation_height = 0; // No activation delay

consensus.vDeployments[Consensus::DEPLOYMENT_WITHDRAWALS].bit = 11;
consensus.vDeployments[Consensus::DEPLOYMENT_WITHDRAWALS].nStartTime = 1704067200; // January 1, 2024
consensus.vDeployments[Consensus::DEPLOYMENT_WITHDRAWALS].nTimeout = Consensus::BIP9Deployment::NO_TIMEOUT;
consensus.vDeployments[Consensus::DEPLOYMENT_WITHDRAWALS].nWindowSize = 120;
consensus.vDeployments[Consensus::DEPLOYMENT_WITHDRAWALS].nThresholdStart = 80; // 80% of 100
consensus.vDeployments[Consensus::DEPLOYMENT_WITHDRAWALS].nThresholdMin = 60; // 60% of 100
consensus.vDeployments[Consensus::DEPLOYMENT_WITHDRAWALS].nFalloffCoeff = 5; // this corresponds to 10 periods
consensus.vDeployments[Consensus::DEPLOYMENT_WITHDRAWALS].useEHF = true;

// The best chain should have at least this much work.
consensus.nMinimumChainWork = uint256S("0x000000000000000000000000000000000000000000000000000000000000000");

Expand Down Expand Up @@ -782,6 +809,15 @@ class CRegTestParams : public CChainParams {
consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].nTimeout = Consensus::BIP9Deployment::NO_TIMEOUT;
consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].min_activation_height = 0; // No activation delay

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].nFalloffCoeff = 5; // this corresponds to 10 periods
consensus.vDeployments[Consensus::DEPLOYMENT_WITHDRAWALS].useEHF = true;

// The best chain should have at least this much work.
consensus.nMinimumChainWork = uint256S("0x00");

Expand Down
1 change: 1 addition & 0 deletions src/consensus/params.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ constexpr bool ValidDeployment(BuriedDeployment dep) { return dep <= DEPLOYMENT_

enum DeploymentPos : uint16_t {
DEPLOYMENT_TESTDUMMY,
DEPLOYMENT_WITHDRAWALS, // Deployment of Fix for quorum selection for withdrawals
// NOTE: Also add new deployments to VersionBitsDeploymentInfo in deploymentinfo.cpp
MAX_VERSION_BITS_DEPLOYMENTS
};
Expand Down
4 changes: 4 additions & 0 deletions src/deploymentinfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ const struct VBDeploymentInfo VersionBitsDeploymentInfo[Consensus::MAX_VERSION_B
/*.name =*/ "testdummy",
/*.gbt_force =*/ true,
},
{
/*.name =*/"withdrawals",
/*.gbt_force =*/true,
},
};

std::string DeploymentName(Consensus::BuriedDeployment dep)
Expand Down
15 changes: 12 additions & 3 deletions src/evo/assetlocktx.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include <chainparams.h>
#include <consensus/params.h>
#include <consensus/validation.h>
#include <deploymentstatus.h>
#include <logging.h>
#include <tinyformat.h>
#include <util/ranges_set.h>
Expand Down Expand Up @@ -113,13 +114,21 @@ bool CAssetUnlockPayload::VerifySig(const llmq::CQuorumManager& qman, const uint
// and at the quorumHash must be active in either the current or previous quorum cycle
// and the sig must validate against that specific quorumHash.


Consensus::LLMQType llmqType = Params().GetConsensus().llmqTypePlatform;

// We check at most 2 quorums
const auto quorums = qman.ScanQuorums(llmqType, pindexTip, 2);
const auto& llmq_params_opt = Params().GetLLMQ(llmqType);
assert(llmq_params_opt.has_value());

// We check two quorums before DEPLOYMENT_WITHDRAWALS activation
// and "all active quorums + 1 the latest inactive" after activation.
const int quorums_to_scan = DeploymentActiveAt(*pindexTip, Params().GetConsensus(), Consensus::DEPLOYMENT_WITHDRAWALS)
? (llmq_params_opt->signingActiveQuorumCount + 1)
: 2;
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-not-active-quorum");
return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-assetunlock-too-old-quorum");
Comment on lines -122 to +131
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why change this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because (active quorums + the latest inactive) are matched. So, quorum can be not-active, but still valid.

? (llmq_params_opt->signingActiveQuorumCount + 1)

}

if (static_cast<uint32_t>(pindexTip->nHeight) < requestedHeight || pindexTip->nHeight >= getHeightToExpiry()) {
Expand Down
1 change: 1 addition & 0 deletions src/rpc/blockchain.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1841,6 +1841,7 @@ RPCHelpMan getblockchaininfo()
SoftForkDescPushBack(tip, softforks, consensusParams, Consensus::DEPLOYMENT_V19);
SoftForkDescPushBack(tip, softforks, consensusParams, Consensus::DEPLOYMENT_V20);
SoftForkDescPushBack(tip, softforks, consensusParams, Consensus::DEPLOYMENT_MN_RR);
SoftForkDescPushBack(tip, ehfSignals, softforks, consensusParams, Consensus::DEPLOYMENT_WITHDRAWALS);
SoftForkDescPushBack(tip, ehfSignals, softforks, consensusParams, Consensus::DEPLOYMENT_TESTDUMMY);

obj.pushKV("softforks", softforks);
Expand Down
50 changes: 47 additions & 3 deletions test/functional/feature_asset_locks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,14 @@
assert_equal,
assert_greater_than,
assert_greater_than_or_equal,
softfork_active,
)
from test_framework.wallet_util import bytes_to_wif

llmq_type_test = 106 # LLMQType::LLMQ_TEST_PLATFORM
tiny_amount = int(Decimal("0.0007") * COIN)
blocks_in_one_day = 576
HEIGHT_DIFF_EXPIRING = 48

class AssetLocksTest(DashTestFramework):
def set_test_params(self):
Expand Down Expand Up @@ -271,6 +273,7 @@ def run_test(self):
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)


def test_asset_locks(self, node_wallet, node, pubkey):
Expand Down Expand Up @@ -345,7 +348,7 @@ def test_asset_unlocks(self, node_wallet, node, pubkey):
asset_unlock_tx_duplicate_index = copy.deepcopy(asset_unlock_tx)
# modify this tx with duplicated index to make a hash of tx different, otherwise tx would be refused too early
asset_unlock_tx_duplicate_index.vout[0].nValue += COIN
too_late_height = node.getblockcount() + 48
too_late_height = node.getblockcount() + HEIGHT_DIFF_EXPIRING

self.check_mempool_result(tx=asset_unlock_tx, result_expected={'allowed': True, 'fees': {'base': Decimal(str(tiny_amount / COIN))}})
self.check_mempool_result(tx=asset_unlock_tx_too_big_fee,
Expand Down Expand Up @@ -430,8 +433,9 @@ def test_asset_unlocks(self, node_wallet, node, pubkey):
self.log.info("Checking that two quorums later it is too late because quorum is not active...")
self.mine_quorum_2_nodes(llmq_type_name='llmq_test_platform', llmq_type=106)
self.log.info("Expecting new reject-reason...")
assert not softfork_active(self.nodes[0], 'withdrawals')
self.check_mempool_result(tx=asset_unlock_tx_too_late,
result_expected={'allowed': False, 'reject-reason' : 'bad-assetunlock-not-active-quorum'})
result_expected={'allowed': False, 'reject-reason' : 'bad-assetunlock-too-old-quorum'})

block_to_reconsider = node.getbestblockhash()
self.log.info("Test block invalidation with asset unlock tx...")
Expand All @@ -445,7 +449,8 @@ def test_asset_unlocks(self, node_wallet, node, pubkey):
self.validate_credit_pool_balance(locked - 2 * COIN)

self.log.info("Forcibly mining asset_unlock_tx_too_late and ensure block is invalid")
self.create_and_check_block([asset_unlock_tx_too_late], expected_error = "bad-assetunlock-not-active-quorum")
assert not softfork_active(self.nodes[0], 'withdrawals')
self.create_and_check_block([asset_unlock_tx_too_late], expected_error = "bad-assetunlock-too-old-quorum")

self.generate(node, 1)

Expand Down Expand Up @@ -636,6 +641,45 @@ def test_mn_rr(self, node_wallet, node, pubkey):
self.generate(node, 1)
assert_equal(locked, self.get_credit_pool_balance())

def test_withdrawal_fork(self, node_wallet, pubkey):
self.log.info("Testing asset unlock after 'withdrawal' activation...")
assert softfork_active(node_wallet, 'withdrawals')

index = 501
while index < 511:
self.log.info(f"Generating new Asset Unlock tx, index={index}...")
asset_unlock_tx = self.create_assetunlock(index, COIN, pubkey)
asset_unlock_tx_payload = CAssetUnlockTx()
asset_unlock_tx_payload.deserialize(BytesIO(asset_unlock_tx.vExtraPayload))

self.log.info("Check that Asset Unlock tx is valid for current quorum")
self.check_mempool_result(tx=asset_unlock_tx, result_expected={'allowed': True, 'fees': {'base': Decimal(str(tiny_amount / COIN))}})

quorumHash_str = format(asset_unlock_tx_payload.quorumHash, '064x')
assert quorumHash_str in node_wallet.quorum('list')['llmq_test_platform']

while quorumHash_str != node_wallet.quorum('list')['llmq_test_platform'][-1]:
self.log.info("Generate one more quorum until signing quorum becomes the last one in the list")
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': True, 'fees': {'base': Decimal(str(tiny_amount / COIN))}})

self.log.info("Generate one more quorum after which signing quorum is gone but Asset Unlock tx is still valid")
assert quorumHash_str in node_wallet.quorum('list')['llmq_test_platform']
self.mine_quorum_2_nodes(llmq_type_name="llmq_test_platform", llmq_type=106)
assert quorumHash_str not in node_wallet.quorum('list')['llmq_test_platform']

if asset_unlock_tx_payload.requestedHeight + HEIGHT_DIFF_EXPIRING > node_wallet.getblockcount():
self.check_mempool_result(tx=asset_unlock_tx, result_expected={'allowed': True, 'fees': {'base': Decimal(str(tiny_amount / COIN))}})
break
else:
self.check_mempool_result(tx=asset_unlock_tx, result_expected={'allowed': False, 'reject-reason' : 'bad-assetunlock-too-late'})
self.log.info("Asset Unlock tx expired, let's try again...")
index += 1

self.log.info("Generate one more quorum after which signing quorum becomes too old")
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'})


if __name__ == '__main__':
AssetLocksTest().main()
11 changes: 11 additions & 0 deletions test/functional/rpc_blockchain.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,17 @@ def _test_getblockchaininfo(self):
'v19': { 'type': 'buried', 'active': False, 'height': 900},
'v20': { 'type': 'buried', 'active': False, 'height': 900},
'mn_rr': { 'type': 'buried', 'active': False, 'height': 900},
'withdrawals': {
'type': 'bip9',
'bip9': {
'status': 'defined',
'start_time': 0,
'timeout': 9223372036854775807, # "withdrawals" does not have a timeout so is set to the max int64 value
'since': 0,
'min_activation_height': 0,
'ehf': True
},
'active': False},
'testdummy': {
'type': 'bip9',
'bip9': {
Expand Down
Loading