New RPC propose and proposetoaddress

[Gnappuraz]

This PR introduces two 2 RPC commands, propose and proposetoaddress their goal is to mimic bitcoin's generate and generatetoaddress, with the difference that they use our new proposing mechanism instead. The goal is to migrate all the calls from all the functional tests to this new RPCs.

[scravy]

Concept ACK

[frolosofsky]

Would be nice to have at least limited test coverage for new rpc functions (e.g. I'm wondering that we can propose number of blocks in a row, I expect "not proposing this time").

[Gnappuraz]

Would be nice to have at least limited test coverage for new rpc functions (e.g. I'm wondering that we can propose number of blocks in a row, I expect "not proposing this time").

Some test coverage is already provided by the functional tests I migrated already. As for the not be able to propose multiple blocks in a row, this is handled by mine_blocks_on_demand that makes it so the difficulty is never checked. I'll add a functional test specific for the rpc.

[pep8speaks]

Hello @Gnappuraz! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers:

Comment last updated at 2019-05-07 10:22:41 UTC

[Gnappuraz]

There're two warnings produced by this PR:

proposer/proposer.cpp:169:30: warning: unused variable 'tip' [-Wunused-variable]
          const CBlockIndex &tip = *m_active_chain->GetTip();

proposer/proposer.cpp:79:9: warning: field 'm_block_builder' will be initialized after field 'm_transaction_picker' [-Wreorder]
        m_block_builder(block_builder),

besides that, ACK 34a28bc

I think the warnings have been fixed.

[scravy]

I have a question: Given that bitcoin removes generate as well as things like signrawtransaction in favor of more specific stuff such as generatetoaddress and signrawtransactionwithkey – does it make sense for us to introduce propose? Eventually it will be more in line with upstream to just have generatetoaddress and proposetoaddress, or not?

[Gnappuraz]

At the moment our propose rpc does not take, like bitcoin a newly generated address and sends the coinbase there, but uses the same script as the coin being used. So I think it does make sense as a simpler version of proposetoaddress. I think sometimes you don't really care to generate a new address to send to coinbase to, this would also reduce the need to create new addresses everytime, making debugging easier sometimes.

[scravy]

At the moment our propose rpc does not take, like bitcoin a newly generated address and sends the coinbase there, but uses the same script as the coin being used

Maybe it would be good to assert that in the functional test which accompanies it?

[Gnappuraz]

as I see you decided to re-order, would be good to have the same order for block_builder and transaction_picker for both proposers

I'm not gonna change the order of other 2 functions in this PR just because they don't have the same parameters order. Order of params cannot be absolute and makes also imho no sense to enforce, adds a useless burden to the development to keep those in the same order.

[AM5800]

Concept ACK 031e49e I started, but didn't finish the review. And unfortunately I won't be able to finish, since I had to leave for a few days now. Overall looks good to me

[scravy]

I am seeing code that I did not see before, seems to have slipped back in when "reverting some changes"?

[scravy]

utACK f626fdf41751af02bbfdc782555eaffb10687a07