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
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

schancel created this revision.Jan 21 2018, 09:12
Herald added a reviewer: Restricted Project. · View Herald TranscriptJan 21 2018, 09:12
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
schancel updated this revision to Diff 2645.Jan 21 2018, 18:56

Add config.h

schancel updated this revision to Diff 2646.Jan 21 2018, 19:25

Add <memory> header.

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
schancel added inline comments.Jan 21 2018, 19:59
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.

schancel updated this revision to Diff 2651.Jan 21 2018, 20:00

Remove config.h

schancel updated this revision to Diff 2652.Jan 21 2018, 20:02

Add config.h back to mining.cpp

deadalnix added inline comments.Jan 21 2018, 20:09
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 ?

schancel updated this revision to Diff 2654.Jan 21 2018, 21:19

Undo changes to Makefile.am

deadalnix accepted this revision.Jan 22 2018, 00:32
This revision is now accepted and ready to land.Jan 22 2018, 00:32
This revision was automatically updated to reflect the committed changes.