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.

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

feat: deprecate RPC 'debug' to favor of 'logging' #6480

Draft
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

knst
Copy link
Collaborator

@knst knst commented Dec 11, 2024

Issue being fixed or feature implemented

The rpc debug duplicates functionality of RPC logging.
Beside that it has next disadvantages:

  • debug doesn't have any tests
  • debug has our own implementation while logging is supported by mainstream
  • logging can work in both modes "all except..." and "only ...", while debug doesn't have a feature "all except..."

Though, there's particular case when debug is more convenient: if you have several categories it's simplier to write debug X+Y rather than logging "[\"X\", \"Y\"]"

Discovered while doing https://github.com/dashpay/dash-issues/issues/63

What was done?

Deprecated rpc debug.
There's some HowTo for debug users for smooth switch to logging:
For debug all: logging [\"all\"]
For debug none: logging '[]' '["all"]'
For debug X+Y: logging "[\"X\", \"Y\"]"

How Has This Been Tested?

Run unit and functional tests

Breaking Changes

It removes RPC debug

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have made corresponding changes to the documentation
  • I have assigned this pull request to a milestone (for repository code-owners and collaborators only)

@knst knst added the RPC Some notable changes to RPC params/behaviour/descriptions label Dec 11, 2024
@knst knst added this to the 23 milestone Dec 11, 2024
@UdjinM6
Copy link

UdjinM6 commented Dec 11, 2024

test  2024-12-11T14:43:28.070000Z TestFramework (INFO): Try running a not whitelisted command as the operator... 
 test  2024-12-11T14:43:28.070000Z TestFramework (ERROR): Assertion failed 
                                   Traceback (most recent call last):
                                     File "/builds/dashpay/dash/build-ci/dashcore-linux64/test/functional/test_framework/test_framework.py", line 161, in main
                                       self.run_test()
                                     File "/builds/dashpay/dash/build-ci/dashcore-linux64/test/functional/rpc_deprecated_platform_filter.py", line 114, in run_test
                                       test_command("debug", ["1"], rpcuser_authpair_operator, 200)
                                     File "/builds/dashpay/dash/build-ci/dashcore-linux64/test/functional/rpc_deprecated_platform_filter.py", line 56, in test_command
                                       assert_equal(resp.status, expected_status)
                                     File "/builds/dashpay/dash/build-ci/dashcore-linux64/test/functional/test_framework/util.py", line 69, in assert_equal
                                       raise AssertionError("not(%s)" % " == ".join(str(arg) for arg in (thing1, thing2) + args))
                                   AssertionError: not(500 == 200)

@UdjinM6
Copy link

UdjinM6 commented Dec 11, 2024

also, should be draft for now to avoid merging before v22.1

@knst knst marked this pull request as draft December 11, 2024 15:31
@knst
Copy link
Collaborator Author

knst commented Dec 12, 2024

 test_command("debug", ["1"], rpcuser_authpair_operator, 200)

This failure will be fixed by merging PR after #6482 (removing this functional test)

I will rebase it when v23 will be close enough

@knst knst requested a review from shumkov December 17, 2024 15:35
@knst
Copy link
Collaborator Author

knst commented Dec 17, 2024

@shumkov we are planning to remove debug RPC in favor of logging which has more features and has upstream support by Bitcoin Core.
I heard dashmate calls debug rpc

If you are using command line option -debug=.... it's fine, no changes required

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RPC Some notable changes to RPC params/behaviour/descriptions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants