Skip to content

[RPC] Register calls where they are defined #1720

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

Jump to bottom
Closed
Liquid369 wants to merge 1 commit into from
Closed

[RPC] Register calls where they are defined #1720

Liquid369 wants to merge 1 commit into from

Conversation

@Liquid369
Copy link
Member

@Liquid369 Liquid369 commented Jun 30, 2020 •
edited
Loading

Follow up to PR #1702
This time it was based off the #1702 branch so it is compiling now.

Bitcoin backport bitcoin#7766

Splitting out registration methods from rpcserver.cpp.

Making for easier RPC command additions and not needing to rebuild everything in rpcserver.h includes.

Split most functions to their respective files,
PIVX specific masternode/budget commands are moved to respective

rpc/masternode.cpp
rpc/budget.cpp

Test functions are now included.

Closing #1714

I believe this is neater and done properly, sorry GIT is a pain sometimes.

EDIT: I see 5 commits urgh, rebase? or.. not sure. I apologize.

@Liquid369 Liquid369 changed the title 2020 register rpc [RPC] Register calls where they are defined Jun 30, 2020
@Liquid369 Liquid369 force-pushed the 2020_register_rpc branch from b33ae32 to acc8345 Compare June 30, 2020 19:51
@furszy
Copy link

furszy commented Jun 30, 2020

Check that you squashed master's recently merged commits inside your commit. Check the code, it has #1715 and #1705 inside.

@Liquid369
Copy link
Member Author

Liquid369 commented Jun 30, 2020

Ah crap.

Suggestion on easy fix?

I am not the best with rebasing/squashing

@furszy
Copy link

furszy commented Jul 1, 2020

  1. git reset HEAD~ --soft
  2. remove the changes included in master manually.
  3. commit the changes again.

@Liquid369
Copy link
Member Author

Liquid369 commented Jul 1, 2020

Did exactly that and it reverting the changes....?

@Liquid369 Liquid369 force-pushed the 2020_register_rpc branch from 905873a to 18de09a Compare July 1, 2020 13:13
@Liquid369
Copy link
Member Author

Liquid369 commented Jul 1, 2020

Alright I got it.

@Liquid369 Liquid369 force-pushed the 2020_register_rpc branch from 82387d9 to b3ca506 Compare July 1, 2020 17:14
@random-zebra
Copy link

random-zebra commented Jul 2, 2020 •
edited
Loading

There are several failures, that should be fixed, in the functional test suite.
Also, this should be rebased on master, now that #1702 has been merged (pull the latest master, then checkout this branch and do git rebase -i master removing the pick lines of the four commits from #1702).

@Liquid369
Copy link
Member Author

Liquid369 commented Jul 2, 2020

I will rebase removing the #1702 commits.

As far as the tests go I have to look and see what specifically failed not familiar with your full test suite.
I see one as a disable wallet issue and I will look into what they are replicating.

Might be some ifdefs from the refactoring missed.

Thanks for the rebasing assistance.

@random-zebra
Copy link

random-zebra commented Jul 2, 2020

@Liquid369 you can check the tests locally, launching the individual test files (from the test/functional directory), or the full test suite with ./test/functional/test_runner.py (first run will take a while to create a cached blockchain, in the successive runs you can reuse it doing ./test/functional/test_runner --keep-cache).

@Liquid369 Liquid369 force-pushed the 2020_register_rpc branch 2 times, most recently from 92872c5 to 04fdc02 Compare July 2, 2020 13:39
@furszy
Copy link

furszy commented Aug 5, 2020

hey, this needs a rebase.
Dunno if you are still planning to work on this or should we take it. Let us know.

@Liquid369
Copy link
Member Author

Liquid369 commented Aug 5, 2020

I plan on finishing, this week I just started getting back into things after the baby. I can rebase off current master? Or should I rebase the prior PR specifically, master should work since merged?

Will try to start today/tomorrow on the rebase and get PR updated.

@Liquid369
Copy link
Member Author

Liquid369 commented Aug 7, 2020

tried rebasing only messing it up more.

@Fuzzbawls Fuzzbawls mentioned this pull request Oct 17, 2020
Merged
random-zebra added a commit that referenced this pull request Oct 30, 2020
@random-zebra
Merge #1926: [RPC][Refactor] Register calls where they are defined
ab460a5 
903b618 Mark findserial and getserials as zerocoin commands (Fuzzbawls)
83dfeaf Refactor: Split up the "pivx" RPC command category (Fuzzbawls)
577414b Refactor: Hide testing-only RPC commands (Fuzzbawls)
a876068 Refactor: Move getstakingstatus to rpcwallet.cpp (Fuzzbawls)
d25ab83 Refactor: Extract zpiv RPC commands out of rpcwallet (Fuzzbawls)
94eb34b [RPC] Register calls where they are defined (Fuzzbawls)

Pull request description:

  Revives/replaces #1720, backporting bitcoin#7766.

  Split out methods to every module, apart from 'help' and 'stop' which
  are implemented in rpcserver.cpp itself.

  - This makes it easier to add or remove RPC commands - no longer
    everything that includes rpcserver.h has to be rebuilt when there's a
    change there.
  - Cleans up `rpc/server.h` by getting rid of the huge cluttered list of
    function definitions.
  - Move zPIV RPC commands to their own file
  - Move `getstakingstatus` to the more appropriate `rpcwallet.cpp`
  - Hide commands intended for testing environments only
  - Split up the ambiguous "pivx" command category

ACKs for top commit:
  furszy:
    looking good, travis is still running but utACK 903b618.
  random-zebra:
    utACK 903b618 and merging...

Tree-SHA512: 84f3340efb6081501277883d3ca10f0c9aff8dbe68859a05bd082471b9a311e834670ab6e773a04e7ecfa665d75f3632e9f66062e65b5a5440b098fef7da2016
@random-zebra
Copy link

random-zebra commented Oct 30, 2020

Replaced by #1926

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@Liquid369 Liquid369

Labels

Refactoring RPC Upstream

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Liquid369 @furszy @random-zebra