From 0350a48ce291096e2f018355e88df4a814d9c771 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Sun, 9 Apr 2023 12:11:22 +0700 Subject: [PATCH] fix: reviewing TODOes at v19 (#5303) During reviewing TODO were found some TODOes that can be done now. - fix: follow-up dash#3467 - replaced commented code to disabled code - follow-up bitcoin#16394 - uncommented code related to `watchonly` feature - removed out-dated TODO in `rpc/masternode.cpp` (already done) - fix: renamed name of clean up test_unittests: removed TODO and updated name of variable TRAVIS - rewritten todo inside `.travis.yml` - fix: adds a missing description for result of rpc `mnsync` Last commit (`mnsync`) is an only candidate for backport to v19, other changes are non significant. Run functional/unit tests No breaking changes - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone --- .travis.yml | 2 +- ci/dash/test_unittests.sh | 3 +-- src/rpc/masternode.cpp | 1 - src/rpc/misc.cpp | 17 +++++++++++++++-- test/functional/feature_dbcrash.py | 19 +++++++++++-------- test/functional/wallet_createwallet.py | 11 +++++------ 6 files changed, 33 insertions(+), 20 deletions(-) diff --git a/.travis.yml b/.travis.yml index a2ed8639d6..242203a557 100644 --- a/.travis.yml +++ b/.travis.yml @@ -176,7 +176,7 @@ before_script: - python3 -c 'import os,sys,fcntl; flags = fcntl.fcntl(sys.stdout, fcntl.F_GETFL); fcntl.fcntl(sys.stdout, fcntl.F_SETFL, flags&~os.O_NONBLOCK);' # Build docker image only for develop branch of the main repo - if [ "$TRAVIS_REPO_SLUG" != "dashpay/dash" -o "$TRAVIS_BRANCH" != "develop" -o "$TRAVIS_PULL_REQUEST" != "false" ]; then export DOCKER_BUILD="false"; echo DOCKER_BUILD=$DOCKER_BUILD; fi - # TODO: Check keys and signed commits + # TODO(ignore if don't use travis): Check keys and signed commits #- if [ "$TRAVIS_REPO_SLUG" = "dashpay/dash" -a "$TRAVIS_PULL_REQUEST" = "false" ]; then while read LINE; do travis_retry gpg --keyserver hkp://subset.pool.sks-keyservers.net --recv-keys $LINE; done < contrib/verify-commits/trusted-keys; fi #- if [ "$TRAVIS_REPO_SLUG" = "dashpay/dash" -a "$TRAVIS_EVENT_TYPE" = "cron" ]; then travis_wait 30 contrib/verify-commits/verify-commits.py; fi after_script: diff --git a/ci/dash/test_unittests.sh b/ci/dash/test_unittests.sh index e7abe0a804..595effffaf 100755 --- a/ci/dash/test_unittests.sh +++ b/ci/dash/test_unittests.sh @@ -16,8 +16,7 @@ if [ "$RUN_UNIT_TESTS" != "true" ]; then exit 0 fi -# TODO this is not Travis agnostic -export BOOST_TEST_RANDOM=1$TRAVIS_BUILD_ID +export BOOST_TEST_RANDOM=${BOOST_TEST_RANDOM:-1} export LD_LIBRARY_PATH=$BASE_BUILD_DIR/depends/$HOST/lib export WINEDEBUG=fixme-all diff --git a/src/rpc/masternode.cpp b/src/rpc/masternode.cpp index 7d4fbb2473..51bd842770 100644 --- a/src/rpc/masternode.cpp +++ b/src/rpc/masternode.cpp @@ -82,7 +82,6 @@ static UniValue masternode_connect(const JSONRPCRequest& request) if (!Lookup(strAddress, addr, 0, false)) throw JSONRPCError(RPC_INTERNAL_ERROR, strprintf("Incorrect masternode address %s", strAddress)); - // TODO: Pass CConnman instance somehow and don't use global variable. NodeContext& node = EnsureNodeContext(request.context); node.connman->OpenMasternodeConnection(CAddress(addr, NODE_NETWORK)); if (!node.connman->IsConnected(CAddress(addr, NODE_NETWORK), CConnman::AllNodes)) diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp index f9763da996..e00c3e92b5 100644 --- a/src/rpc/misc.cpp +++ b/src/rpc/misc.cpp @@ -79,8 +79,21 @@ static UniValue mnsync(const JSONRPCRequest& request) { {"mode", RPCArg::Type::STR, RPCArg::Optional::NO, "[status|next|reset]"}, }, - RPCResults{},/*TODO*/ - RPCExamples{""} + { + RPCResult{"for mode = status", + RPCResult::Type::OBJ, "", "", + { + {RPCResult::Type::NUM, "AssetID", "The asset ID"}, + {RPCResult::Type::STR, "AssetName", "The asset name"}, + {RPCResult::Type::NUM, "AssetStartTime", "The asset start time"}, + {RPCResult::Type::NUM, "Attempt", "The attempt"}, + {RPCResult::Type::BOOL, "IsBlockchainSynced", "true if the blockchain synced"}, + {RPCResult::Type::BOOL, "IsSynced", "true if synced"}, + }}, + RPCResult{"for mode = next|reset", + RPCResult::Type::STR, "", ""}, + }, + RPCExamples{""}, }.Check(request); std::string strMode = request.params[0].get_str(); diff --git a/test/functional/feature_dbcrash.py b/test/functional/feature_dbcrash.py index 4e99eb5992..01078fe2a1 100755 --- a/test/functional/feature_dbcrash.py +++ b/test/functional/feature_dbcrash.py @@ -232,6 +232,8 @@ class ChainstateWriteCrashTest(BitcoinTestFramework): # Syncing the blocks could cause nodes to crash, so the test begins here. self.sync_node3blocks(block_hashes_to_sync) + starting_tip_height = self.nodes[3].getblockcount() + # Main test loop: # each time through the loop, generate a bunch of transactions, # and then either mine a single new block on the tip, or some-sized reorg. @@ -242,14 +244,15 @@ class ChainstateWriteCrashTest(BitcoinTestFramework): # Pick a random block between current tip, and starting tip current_height = self.nodes[3].getblockcount() # TODO: re-enable this when ReplayBlocks is fixed to support evodb and additional indexes - # random_height = random.randint(starting_tip_height, current_height) - # self.log.debug("At height %d, considering height %d", current_height, random_height) - # if random_height > starting_tip_height: - # # Randomly reorg from this point with some probability (1/4 for - # # tip, 1/5 for tip-1, ...) - # if random.random() < 1.0 / (current_height + 4 - random_height): - # self.log.debug("Invalidating block at height %d", random_height) - # self.nodes[3].invalidateblock(self.nodes[3].getblockhash(random_height)) + skip_this_test_ReplayBlocks = True + random_height = random.randint(starting_tip_height, current_height) + self.log.debug("At height %d, considering height %d", current_height, random_height) + if not skip_this_test_ReplayBlocks and random_height > starting_tip_height: + # Randomly reorg from this point with some probability (1/4 for + # tip, 1/5 for tip-1, ...) + if random.random() < 1.0 / (current_height + 4 - random_height): + self.log.debug("Invalidating block at height %d", random_height) + self.nodes[3].invalidateblock(self.nodes[3].getblockhash(random_height)) # Now generate new blocks until we pass the old tip height self.log.debug("Mining longer tip") diff --git a/test/functional/wallet_createwallet.py b/test/functional/wallet_createwallet.py index 3e90d9cead..cc00c3f9f0 100755 --- a/test/functional/wallet_createwallet.py +++ b/test/functional/wallet_createwallet.py @@ -123,12 +123,11 @@ class CreateWalletTest(BitcoinTestFramework): w7 = node.get_wallet_rpc('w7') assert_raises_rpc_error(-15, 'Error: running with an unencrypted wallet, but walletpassphrase was called.', w7.walletpassphrase, '', 10) - # TODO: renable this when avoid reuse flag is added - # self.log.info('Test making a wallet with avoid reuse flag') - # self.nodes[0].createwallet('w8', False, False, '', True) # Use positional arguments to check for bug where avoid_reuse could not be set for wallets without needing them to be encrypted - # w8 = node.get_wallet_rpc('w8') - # assert_raises_rpc_error(-15, 'Error: running with an unencrypted wallet, but walletpassphrase was called.', w7.walletpassphrase, '', 10) - # assert_equal(w8.getwalletinfo()["avoid_reuse"], True) + self.log.info('Test making a wallet with avoid reuse flag') + self.nodes[0].createwallet('w8', False, False, '', True) # Use positional arguments to check for bug where avoid_reuse could not be set for wallets without needing them to be encrypted + w8 = node.get_wallet_rpc('w8') + assert_raises_rpc_error(-15, 'Error: running with an unencrypted wallet, but walletpassphrase was called.', w7.walletpassphrase, '', 10) + assert_equal(w8.getwalletinfo()["avoid_reuse"], True) self.log.info('Using a passphrase with private keys disabled returns error') assert_raises_rpc_error(-4, 'Passphrase provided but private keys are disabled. A passphrase is only used to encrypt private keys, so cannot be used for wallets with private keys disabled.', self.nodes[0].createwallet, wallet_name='w9', disable_private_keys=True, passphrase='thisisapassphrase')