Skip to content

[Refactor] decouple wallet rpc commands #5934

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
wants to merge 5 commits into from
Closed

[Refactor] decouple wallet rpc commands #5934

wants to merge 5 commits into from

Conversation

jonasschnelli
Copy link
Contributor

includes #5744

@jonasschnelli jonasschnelli force-pushed the 2015/03/wallet_rpc_flex branch from 674fc82 to 07c3027 Compare March 21, 2015 22:01
@jonasschnelli
Copy link
Contributor Author

I think this PR's implementation is not perfect and is not very modular. But it would give the opportunity to slowly decouple the wallet from the rest of the core.
It's unclear why we have a runtime linking of rpc commands and there are probably way better solutions of a dynamic handling of the rpc commands dispatching.
But these are out of the PRs focus.

Longterm goals to achieve with this PR:

  • Remove/reduce wallet_enabled #ifdefs
  • Possibility to switch wallet implementation on compile and/or runtime (as startup/config arg)

fanquake
fanquake reviewed Mar 23, 2015
View reviewed changes
src/rpcclient.h
/**
* Add new conversion to the JSON RPC conversion table
* @param method name of the method
* @param idx 0-based index of the paramter
Copy link
Member

Choose a reason for hiding this comment

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

s/paramter/parameter

@laanwj laanwj added the Wallet label Mar 26, 2015
@jonasschnelli jonasschnelli force-pushed the 2015/03/wallet_rpc_flex branch from 07c3027 to d41f25a Compare April 12, 2015 09:32
@jonasschnelli
Copy link
Contributor Author

Rebased

@jonasschnelli jonasschnelli mentioned this pull request Apr 24, 2015
Closed
@jtimon
Copy link
Contributor

jtimon commented Jun 21, 2015

Needs rebase

@jonasschnelli
Copy link
Contributor Author

@jtimon: will do soon. But maybe this PR #6006 makes more sense. What do you think?

This PR would need a better locking mechanism.

jonasschnelli and others added 5 commits June 23, 2015 21:55
@jonasschnelli
[Modularization] add possibility to extend or overwrite RPC calls
fb2db93 
- this will allow a upcomming modularization of the wallet or other components
@dexX7 @jonasschnelli
[Modularization] allow to add entries to the JSON conversion table
326dce8 
- add method to insert custom conversion entries

Conflicts:
	src/test/rpc_tests.cpp
@jonasschnelli
[Modularization] flexible RPC table overhaul
0e0f562
@dexX7 @jonasschnelli
[Modularization] use pointers when adding or replacing RPC commands
f3fb24e 
Using pointers signals the responsibility of managing object lifetime is in the hand of the caller.
@jonasschnelli
[Refactor] decouple wallet rpc commands
a6b659a 
includes bitcoin#5744
@jonasschnelli jonasschnelli force-pushed the 2015/03/wallet_rpc_flex branch from d41f25a to a6b659a Compare June 24, 2015 06:43
@jonasschnelli
Copy link
Contributor Author

Rebased.
Misses a clever locking for CRPCTable tableRPC.
The signaling solution (#6006) which supersedes this PR is better IMO.

@jonasschnelli
Copy link
Contributor Author

Closing in favor of #6006

@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
Wallet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@jonasschnelli @jtimon @fanquake @laanwj @dexX7