Page MenuHomePhabricator

[backport] rpc: Move the `generate` RPC call to rpcwallet
ClosedPublic

Authored by schancel on Jan 21 2018, 09:12.

Details

Summary

Backport Core PR10683: rpc: Move the generate RPC call to rpcwallet

2a96283 rpc: Update `generate` for developer notes (Wladimir J. van der Laan)
df7e2f0 rpc: Move the `generate` RPC call to rpcwallet (Wladimir J. van der Laan)

This makes it possible to mine to any wallet when multi-wallet mode is added.
Solves the same problem as #10649, but IMO in a cleaner way.

It also gets rid of the circuitous ScriptForMining method on
CValidationInterface, which really doesn't belong there.

After this change it's still possible to mine without wallet through
generatetoaddress.

Closes T186

Test Plan

make check && ./test/functional/test_runner.py

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

deadalnix requested changes to this revision.Jan 21 2018, 13:28

This doesn't even build.

This revision now requires changes to proceed.Jan 21 2018, 13:28
deadalnix requested changes to this revision.Jan 21 2018, 19:53
deadalnix added inline comments.
src/Makefile.am
229 ↗(On Diff #2646)

Why ?

src/rpc/mining.h
8 ↗(On Diff #2646)

Remove. You are only using Config by reference, you don't need it.

This revision now requires changes to proceed.Jan 21 2018, 19:53
src/Makefile.am
229 ↗(On Diff #2646)

Dependencies to RPC server stuff. The wallet library shouldn't be including RPC handlers, and they fail to compile now unless you also include rpc/mining.cpp down in the wallet.

Add config.h back to mining.cpp

src/Makefile.am
229 ↗(On Diff #2646)

Looking there, it doesn't looks like it moved: https://github.com/bitcoin/bitcoin/pull/10683/files

It also do not seems to make a lot of sense to include the RPC wallet in there. What happen if we build without wallet support ?

Undo changes to Makefile.am

This revision is now accepted and ready to land.Jan 22 2018, 00:32
schancel added a task: Restricted Maniphest Task.
This revision was automatically updated to reflect the committed changes.