Add the query_txid_plugin as a option on the develop branch

[bijianing97]

The pull request is the solution for the #1954, I rebased on the current develop.

[oxarbitrage]

Looks good, i am going to test it and comment/review further.

In the meantime if you want to fix some minor style adjustments i sent will be great. Thanks!

[bijianing97]

ok,I will fix them and should I create a new pull request?

[bijianing97]

I have fixed the above questions and hope you will like it.

[oxarbitrage]

Thank you @bijianing97. Looking good.

I noticed that the new plugin files like https://github.com/bitshares/bitshares-core/blob/32ba6bbf1f2ee9b7b781ac7bf53d2eeae1c01cbf/libraries/plugins/query_txid/query_txid_plugin.cpp the indentation is 4 spaces while the Bitshares project use 3 (almost) everywhere. Will be good if you can change these.

I sent also a few more comments. Adding more commits into this pull request is fine and what you should do, by now your commits have all the same name. Will be good if next ones haves a more descriptive name at least to differentiate them.

Overall great work, i am downloading the requirements and testing further. Will send some more comments once there.

[bijianing97]

OK，I will try to keep it in few days because i have other work to do

[oxarbitrage]

There are some compiler warnings about signed/unsigned comparisons that i think it should be easy to remove:

[ 64%] Building CXX object libraries/plugins/query_txid/CMakeFiles/graphene_query_txid.dir/query_txid_plugin.cpp.o
/home/oxarbitrage/CLionProjects/pull1957/bitshares-core/libraries/plugins/query_txid/query_txid_plugin.cpp: In member function ‘void graphene::query_txid::detail::query_txid_plugin_impl::collect_txid_index(const graphene::protocol::signed_block&)’:
/home/oxarbitrage/CLionProjects/pull1957/bitshares-core/libraries/plugins/query_txid/query_txid_plugin.cpp:102:30: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
       for (auto idx = 0; idx < b.transactions.size(); idx++) {
                          ~~~~^~~~~~~~~~~~~~~~~~~~~~~
/home/oxarbitrage/CLionProjects/pull1957/bitshares-core/libraries/plugins/query_txid/query_txid_plugin.cpp: In member function ‘void graphene::query_txid::detail::query_txid_plugin_impl::consume_block()’:
/home/oxarbitrage/CLionProjects/pull1957/bitshares-core/libraries/plugins/query_txid/query_txid_plugin.cpp:128:21: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
       while (number > limit_batch) {
              ~~~~~~~^~~~~~~~~~~~~
/home/oxarbitrage/CLionProjects/pull1957/bitshares-core/libraries/plugins/query_txid/query_txid_plugin.cpp:131:33: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
          for (auto idx = 0; idx < limit_batch; idx++) {
                             ~~~~^~~~~~~~~~~~~
/home/oxarbitrage/CLionProjects/pull1957/bitshares-core/libraries/plugins/query_txid/query_txid_plugin.cpp:149:21: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
       if (backupnum > limit_batch)
           ~~~~~~~~~~^~~~~~~~~~~~~
[ 64%] Linking CXX static library libgraphene_query_txid.a
[ 64%] Built target graphene_query_txid

[bijianing97]

I modify some variables from signed to unsigned and do not get warning about that.And show the hint about query_txid like the UES_PROFILER.

[bijianing97]

ok, i fixed it.

[oxarbitrage]

Good, thanks for the contribution, very valuable in my opinion for its speed. We can make further plugins using it.
Thank you also for taking the time to fix the details.

[pmconrad]

Thanks!
I think we should enable the plugin in travis and docker builds. Can you add that to .travis.yml and Dockerfile (don't forget to install the new dependencies)?

[pmconrad]

The use of signals here looks unnecessarily complicated to me. You only connect one consumer to each signal, and trigger each signal only in one place. Why not call the consumer there directly?

[bijianing97]

I add the plugin to .travis.yml and Dockerfile ,but failed in the docker ,i can’t see the details and modify it

[pmconrad]

Please add some unit tests. Need a separate test suite though that gets only built if the plugin is enabled.

[pmconrad]

If the plugin is not enabled the method doesn't return anything. IMO it should throw.

[pmconrad]

*itor is undefined if itor == txid_index.end()

[pmconrad]

but failed in the docker ,i can’t see the details and modify it

Currently it fails because the container doesn't have wget.
But it makes no sense to implement this blindly, if you don't have a local docker installation for testing you can leave it out.

[bijianing97]

I'm sorry that I don't know how to add some unit tests, could you give me some example, I will try to do it in few days.

[pmconrad]

Here's an example for a unit test suite: https://github.com/bitshares/bitshares-core/blob/3.3.1/tests/performance/performance_tests.cpp

[pmconrad]

I think this is superfluous. The library dependency above should make the plugin include files available to app.

[pmconrad]

Now I get it. You need the include even when the plugin is disabled, because it declares the structure that is used in the return value of the API calls.

You can work around this by typedefing query_trx_info to some other known structure in that case (the API call should always throw an error, so the actual return type will not be used).

[pmconrad]

Please let precompiler directives always start at the first column, don't indent them.

[pmconrad]

There's a space missing after =ON.