< sipa>
jonasschnelli: no, i don't understand why it can be correct
< sipa>
i'm fixing it now
< sipa>
(but maybe i'm missing something)
< sipa>
so feel free to explain why you think it is
< paveljanik>
jonasschnelli, wumpus: in src/rpc, we shadow global tableRPC by the same-named argument to all Register*RPCCommands(CRPCTable &table). Before changing all names (by adding underscore), do we really want to pass the global in and then shadow it by the same named argument? What about removing the argument instead?
< sipa>
how about renaming the global? :)
< paveljanik>
there are millions of use of it outside the register part...
< sipa>
well, in an ideal world there would not be a global (or at least not one that's commonly used)
< paveljanik>
sipa, I prefer to rename argument names
< sipa>
paveljanik: ok
< paveljanik>
to e.g. t ;-)
< paveljanik>
to save a lot of bytes ;-)
< sipa>
paveljanik: an alternative is making the tableRPC global static (and not exposed from rpc/server.cpp), and instead add a function that just calls tableRPC.appendCommand
< sipa>
it's not shadowing if the global isn't visible
< paveljanik>
this can't work: qt/rpcconsole.cpp: UniValue result = tableRPC.execute(
< sipa>
ok, a wrapper for that too :)
< paveljanik>
and now clang-fu it ;-)
< wumpus>
sipa: well the idea is to make it possible to have multiple rpc tables
< wumpus>
sipa: that's why tablerpc is an argument to Register* in the first place
< paveljanik>
yes
< wumpus>
this makes it possible to have multiple RPC entry points, at some point
< jonasschnelli>
Yes. The idea of the register is to avoid a superglobal dispatch table.
< wumpus>
probably it shouldn't be a global though
< wumpus>
agree with that
< wumpus>
but please don't hardcode tablerpc
< jonasschnelli>
The table itself should be a rpc-server object.
< jonasschnelli>
Modules should "register" callbacks.
< paveljanik>
I'm most inclned to rename arguments to t...
< wumpus>
in any case, let's not entangle this with the refactor to remove shadowing
< GitHub19>
bitcoin/master a946bb6 Jonas Schnelli: [RPC] createrawtransaction: add option to set the sequence number per input
< GitHub19>
bitcoin/master e59336f Jonas Schnelli: [bitcoin-tx] allow to set nSequence number over the in= command
< GitHub19>
bitcoin/master ae357d5 Jonas Schnelli: [Bitcoin-Tx] Add tests for sequence number support
< GitHub140>
[bitcoin] laanwj closed pull request #7957: [RPC][Bitcoin-TX] Add support for sequence number (master...2016/04/rbf_base) https://github.com/bitcoin/bitcoin/pull/7957
< GitHub110>
[bitcoin] paveljanik opened pull request #8163: Do not shadow global RPC table variable (tableRPC) (master...20160607_shadowing_rpc) https://github.com/bitcoin/bitcoin/pull/8163
< jonasschnelli>
I guess the only way is by trying it yourself. :)
< jonasschnelli>
But GPU mining is history? right?
< jonasschnelli>
(at least on the bitcoin blockchain)
< Danco_>
For Litecoin is still useful.
< btcdrak>
Danco_: it's probably a question for #bitcoin-dev or #bitcoin. this channel is specifically for Bitcoin Core development.
< Danco_>
Ok ok i'll make it there :D thanks.
< GitHub70>
[bitcoin] paveljanik opened pull request #8166: src/test: Do not shadow local variables (master...20160607_shadowing_tests) https://github.com/bitcoin/bitcoin/pull/8166
< cfields_>
jonasschnelli: right, probably low on the list of real concerns
< cfields_>
jonasschnelli: sec, i'm looking up the actual failure. I'd hate mask real bugs there.
< cfields_>
jonasschnelli: ok, just tested. It's pretty easy to screw that up, especially if you're typing a seq by hand and add one too many digits. Silently inserting the wrong value is pretty nasty.
< sipa>
luke-jr: good thing BIP8 was never assigned https://en.wikipedia.org/wiki/BIP-8 (image what would happen if something related to error correction or checksums got that...)