Daemon SonarCloud #269

cartoon-face · 2022-02-04T14:46:47Z

Additions

CRYPTONOTE_NAME has been changed to BLOCKCHAIN_DIR as the main reason this is used is to name the blockchain folder when the daemon is launched
CCX_RELEASE_VERSION + CCX_WALLET_RELEASE_VERSION additions to form a string saying "Conceal (or wallet) v" + version + (ID) I.E "Conceal v6.6.1 (Ataegina)"
print_genesis_tx_hex() will now only print a random hash, the user who is using this command should know what to do with it, we're not here to teach
made use of 2 already implemented functions, print_bci and print_bc_outs <file_name>
made use of &args in every command by using it for usage checks
added debugging logging on commands (and functions in this file), maybe they'll prove useful

Removals

remove commented code
remove timestamps and log level from client, keep them to the log file
removed renameDataDir() as I think this comes from the cryptonote era as as far as i know, conceal has never been called anything different
removed all previous calls of std::cout to respectively run whatever string through the logger

Ways to Test

How does the Conceal version look when you load the daemon?

It should look like: Conceal v6.6.1 (Ataegina).
Please report anything different.

What does the commands print_bci and print_bc_outs <file_name> do?

print_bci should print out the current blockchain height.
print_bc_outs should print out the outputs in the current block.
Please report anything different.

Does everything logged on screen get logged in conceald.log folder?

Please report if not.

What happens if you try and use an arg where one is not needed?

The daemon should inform you of the proper usage for all commands.
Please report anything different.

Are the client changes regarding removing the timestamps and log level good?

In terms of looks, it works how concealwallet works.
(Yes/No)

* CRYPTONOTE_NAME has been changed to BLOCKCHAIN_DIR as the main reason this is used is to name the blockchain folder when the daemon is launched * CCX_RELEASE_VERSION + CCX_WALLET_RELEASE_VERSION additions to form a string saying "Conceal (or wallet) v" + version + (ID) * `print_genesis_tx_hex()` will now only print a random hash, the user who is using this command should know what to do with it, we're not here to teach * remove commented code * pretty daemon - remove timestamps and log level from client, keep them to the log file * removed `renameDataDir()` as I think this comes from the cryptonote era as as far as i know, conceal has never been called anything different * removed all previous calls of `std::cout` to respectively run whatever string through the logger * made use of 2 already implemented functions, `print_bci` and `print_bc_outs <file_name>` * made use of &args in every command by using it for usage checks * added debugging logging on commands (and functions in this file), maybe they'll prove useful *

Not sure why to_string was called for a string

krypt0x

Everything seems fine. Thank you

krypt0x · 2022-02-10T14:09:57Z

still waiting for @AxVultis review

btw, we will need to rebase this commit because in the meantime we had an urgent PR that needed to be merged ASAP

* we use this function as a boolean, just in case in the future we ever do need to list all the block hashes. * updated daemon command to false on print blockchain indexs so it will print the id of the current height and not an array from 0 to current

cartoon-face · 2022-02-11T00:33:41Z

I've updated the logic behind the print_bci and turned this into a boolean. If set true, it will print all block index's from 0 to the current, else if set to false it will only print the current id and height.

For the purpose of this daemon command, it has been set to false as we don't want to overload the daemon with this one request. A fully synced chain would print over 900k+ of block ids. The logic has been changed to still be able to do that just in case it is ever needed in the future.

I've also upstreamed this commit to include the recent work from the development branch. Should be good to go.

I can't seem to figure a way for this command to print out only the information for a given height, I suggest we re-remove this command for now. This is a debug command to be fair and shouldn't really be needed by the user

AxVultis

Great job

AxVultis · 2022-02-05T15:36:48Z

src/Daemon/Daemon.cpp

@@ -43,12 +43,12 @@ namespace po = boost::program_options;

 namespace
 {
-  const command_line::arg_descriptor<std::string> arg_config_file = {"config-file", "Specify configuration file", std::string(cn::CRYPTONOTE_NAME) + ".conf"};
+  const command_line::arg_descriptor<std::string> arg_config_file = {"config-file", "Specify configuration file", "Conceal.conf"};


I think we should keep the default to conceal.conf (with lower-case c)

AxVultis · 2022-02-05T15:39:16Z

src/Daemon/Daemon.cpp

+      if (!config_path.has_parent_path())
+        config_path = data_dir_path / config_path;
+
+      boost::system::error_code ec;
+      if (boost::filesystem::exists(config_path, ec))
+        po::store(po::parse_config_file<char>(config_path.string<std::string>().c_str(), desc_cmd_sett), vm);


I would suggest keeping braces (here and everywhere) even for single lines.

AxVultis · 2022-02-05T15:46:24Z

src/Daemon/DaemonCommandsHandler.cpp


  uint16_t l = 0;
  if (!common::fromString(args[0], l))
  {
-    std::cout << "wrong number format, use: set_log <log_level_number_0-4>" << ENDL;
+    logger(logging::ERROR) << "Incorrect number format, use: set_log <log_level_number_0-4>";


Suggested change

logger(logging::ERROR) << "Incorrect number format, use: set_log <log_level_number_0-4>";

logger(logging::ERROR) << "Incorrect number format, use: set_log <0-4> (0-4 being log level)";

src/Daemon/DaemonCommandsHandler.cpp

* taken from the review of this opened pr. * EDIT: Added BUILD_COMMIT_ID as a debugging log next to version. * if we want a user who is maybe having trouble to produce the exact version, then loading the daemon with the version flag will produce the ID without the need to change the log level within the daemon to find it out

Accidental double ops “<<“

krypt0x · 2022-02-14T11:12:41Z

What's the status of this PR?

cartoon-face · 2022-02-15T22:49:42Z

What's the status of this PR?

Just need to make sure print_as_json is printing the correct information. I have reverted the changes regarding this function so that should work like it used to.

It needs new review

krypt0x

It's failing on build checks. I'm afraid this PR needs to be rebased with development branch again.

cartoon-face · 2022-02-16T15:56:23Z

It's failing on build checks. I'm afraid this PR needs to be rebased with development branch again.

Upstreamed the forked development branch, ready for testing again

Matches BLOCKCHAIN_DIR[] original state

mainzerp · 2022-02-17T10:58:30Z

Looks like Conceal v6.6.1 (Ataegina) - Yes
print_bci works fine now.
print_bc_outs test.txt prints "Unknown command: print_bc_outs".
User commands doesn't log to conceald.log.
Using unecessary arg print_cn 4: 11:55:51.049590 ERROR Usage: "print_cn".
Looks and works like concealwallet - Yes

cartoon-face · 2022-02-17T16:26:53Z

Looks like Conceal v6.6.1 (Ataegina) - Yes

print_bci works fine now.
print_bc_outs test.txt prints "Unknown command: print_bc_outs".

User commands doesn't log to conceald.log.

Using unecessary arg print_cn 4: 11:55:51.049590 ERROR Usage: "print_cn".

Looks and works like concealwallet - Yes

Thanks for testing this @mainzerp! Your efforts are greatly appreciated with helping improve Conceals core software

Side notes:

I removed print_bc_outs since first opening this PR so we don't have to worry about that here.
Timestamps and log levels have been put back into the daemon client.

krypt0x · 2022-02-17T16:34:05Z

Looks like Conceal v6.6.1 (Ataegina) - Yes

print_bci works fine now.
print_bc_outs test.txt prints "Unknown command: print_bc_outs".

User commands doesn't log to conceald.log.

Using unecessary arg print_cn 4: 11:55:51.049590 ERROR Usage: "print_cn".

Looks and works like concealwallet - Yes

Thanks for the feedback

krypt0x

Everything seems alright on my side

cartoon-face and others added 2 commits February 4, 2022 14:41

Fix tests

99e0c06

Not sure why to_string was called for a string

krypt0x previously approved these changes Feb 9, 2022

View reviewed changes

krypt0x requested a review from AxVultis February 9, 2022 12:48

krypt0x self-assigned this Feb 9, 2022

krypt0x added the enhancement New feature or request label Feb 9, 2022

cartoon-face and others added 2 commits February 10, 2022 17:14

Merge branch 'ConcealNetwork:development' into development

7068b6e

print_bc_outs removal

ee36891

I can't seem to figure a way for this command to print out only the information for a given height, I suggest we re-remove this command for now. This is a debug command to be fair and shouldn't really be needed by the user

AxVultis reviewed Feb 12, 2022

View reviewed changes

cartoon-face and others added 2 commits February 13, 2022 02:50

Update Daemon.cpp

3f55303

Accidental double ops “<<“

cartoon-face added 2 commits February 15, 2022 22:40

Merge branch 'ConcealNetwork:development' into development

d3872f1

Revert print_as_json changes

2e1da49

krypt0x self-requested a review February 15, 2022 23:24

krypt0x requested a review from AxVultis February 15, 2022 23:25

krypt0x reviewed Feb 16, 2022

View reviewed changes

krypt0x self-requested a review February 16, 2022 12:43

Merge branch 'ConcealNetwork:development' into development

41c6efc

Lowercase this c

91b0627

Matches BLOCKCHAIN_DIR[] original state

krypt0x approved these changes Feb 21, 2022

View reviewed changes

krypt0x merged commit bbca5e3 into ConcealNetwork:development Feb 21, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Daemon SonarCloud #269

Daemon SonarCloud #269

cartoon-face commented Feb 4, 2022 •

edited

Loading

krypt0x left a comment

krypt0x commented Feb 10, 2022

cartoon-face commented Feb 11, 2022

AxVultis left a comment

AxVultis Feb 5, 2022

AxVultis Feb 5, 2022

AxVultis Feb 5, 2022

krypt0x commented Feb 14, 2022

cartoon-face commented Feb 15, 2022

krypt0x left a comment

cartoon-face commented Feb 16, 2022

mainzerp commented Feb 17, 2022

cartoon-face commented Feb 17, 2022

krypt0x commented Feb 17, 2022

krypt0x left a comment

	logger(logging::ERROR) << "Incorrect number format, use: set_log <log_level_number_0-4>";
	logger(logging::ERROR) << "Incorrect number format, use: set_log <0-4> (0-4 being log level)";

Daemon SonarCloud #269

Daemon SonarCloud #269

Conversation

cartoon-face commented Feb 4, 2022 • edited Loading

Additions

Removals

Ways to Test

krypt0x left a comment

Choose a reason for hiding this comment

krypt0x commented Feb 10, 2022

cartoon-face commented Feb 11, 2022

AxVultis left a comment

Choose a reason for hiding this comment

AxVultis Feb 5, 2022

Choose a reason for hiding this comment

AxVultis Feb 5, 2022

Choose a reason for hiding this comment

AxVultis Feb 5, 2022

Choose a reason for hiding this comment

krypt0x commented Feb 14, 2022

cartoon-face commented Feb 15, 2022

krypt0x left a comment

Choose a reason for hiding this comment

cartoon-face commented Feb 16, 2022

mainzerp commented Feb 17, 2022

cartoon-face commented Feb 17, 2022

krypt0x commented Feb 17, 2022

krypt0x left a comment

Choose a reason for hiding this comment

cartoon-face commented Feb 4, 2022 •

edited

Loading