< luke-jr>
sipa: then adapt named arguments to work nicer with options? the current way is a kludge, and we shouldn't let named arguments detract from a good non-named interface
< sipa>
luke-jr: care to elaborate?
< kallewoof>
(JSON and *sh don't really go well together as it is. '{"address":"$myaddr"}--- oh wait I need to use " or $ won't resolve. "{'addr... oh wait JSON won't allow apos strings.. gaah... etc.)
< esotericnonsense>
is it possible to explicitly specify named arguments in json queries? I dislike knowing that when writing an API, the commands could change out from under me and I wouldn't know. (yes, modifications to the RPC calls are designed such that it shouldn't happen, but it's still unnerving)
< sipa>
esotericnonsense: yes, JSON RPC supports named arguments
< sipa>
where the arguments are not an array but an object
< esotericnonsense>
for all calls? that's awesome. argh, /me goes off to read the manual.
< sipa>
yes, for all calls
< sipa>
(since 0.13 or 0.14 it's implemented in bitcoin core, i believe)
< luke-jr>
sipa: tacking on a bunch of optional ordered arguments is unwieldy and a terrible interface; for such cases, an options object makes sense
< sipa>
luke-jr: an options object is also an unwieldy and terrible interface
< sipa>
(see kallewoof's comment above)
< luke-jr>
sipa: that's an issue with sh, not JSON-RPC
< sipa>
luke-jr: programmatic use of JSON-RPC should just upgrade to named arguments, IMHO
< sipa>
safer, clearer, and more flexible in every way
< sipa>
the only downside to named arguments i think is in CLI usage on the command line, where "-named ... name1= ... name2= ..." need to be added
< luke-jr>
some languages don't even support named arguments
< sipa>
then write a wrapper - invoking that will at worst be as hard as constructing the options object you'd otherwise pass
< luke-jr>
sipa: this is not an argument to have a crappy interface
< sipa>
luke-jr: indeed it isn't, but i know of no solution to that problem
< sipa>
having many parameters to an RPC will always be annoying to use
< luke-jr>
the options object is the solution we've used so far
< sipa>
and i claim that named arguments are in no way worse than an options object
< sipa>
(in some ways it's equally terribly, though)
< luke-jr>
even if it were true, that's irrelevant, since the topic is the ordered params API
< sipa>
i don't think that's a very interesting topic
< sipa>
when a better alternative exists
< sipa>
in every situation where you'd call an RPC with an options object, have JSON-RPC wrapper that takes a JSON object as argument, and treats its key-value pairs as names and values of named arguments in a call
< esotericnonsense>
submitted as #1828 on the bitcoin.org repo. probably just me that's missed it having not been around for the releases but it's nice to have in there.
< esotericnonsense>
no gribble. (probably the wrong channel anyway).
< bitcoin-git>
[bitcoin] luke-jr opened pull request #11441: rpc/server: Support for specifying options as named parameters (master...rpc_namedopts) https://github.com/bitcoin/bitcoin/pull/11441
< bitcoin-git>
[bitcoin] fanquake opened pull request #11442: [WIP] [Docs] Update OpenBSD Build Instructions for OpenBSD 6.1 (master...openbsd-doc-update) https://github.com/bitcoin/bitcoin/pull/11442
< bitcoin-git>
bitcoin/master 5ab586f James O'Beirne: Consolidate CMerkleBlock constructor into a single method...
< bitcoin-git>
bitcoin/master 46ce223 James O'Beirne: Add tests for CMerkleBlock usage with txids specified
< bitcoin-git>
bitcoin/master dbc4ae0 MarcoFalke: Merge #11293: Deduplicate CMerkleBlock construction code, add test coverage...
< bitcoin-git>
[bitcoin] MarcoFalke closed pull request #11293: Deduplicate CMerkleBlock construction code, add test coverage (master...dedup-cmerkleblock) https://github.com/bitcoin/bitcoin/pull/11293
< BlueMatt>
jonasschnelli: hmm, I thought you were going to always set NODE_NETWORK_LIMITED?
< BlueMatt>
ie whether pruned or not just set it
< jnewbery>
I'm late to the promag BlueMatt zmq party, but my view is that there are no docs stating that block/tx/wallet notifications are sent in any particular order, therefore our tests shouldn't expect a particular ordering
< BlueMatt>
jnewbery: I think our tests should test for what clients may be relying on
< BlueMatt>
and I think it would be reasonable(ish) for a client to rely on ordering
< BlueMatt>
(but, more importantly, it doesnt matter much atm...keeping the ordering consistent across 0.16 should be no problem, and we can follow a deprecation window if we write up some docs easily)
< jj_>
hi
< jnewbery>
BlueMatt: to be clear, are you talking about ordering within a notification type (ie blocks are published in the order they arrive), or between notification types (ie the hashtx notification always comes before the rawtx notification)?
< BlueMatt>
jnewbery: between notification types
< jnewbery>
ok, in that case I disagree :)
< jnewbery>
clients that are relying on that ordering rather than checking the command/topic are broken
< jnewbery>
that said, I fully support documenting all this!
< BlueMatt>
why shouldnt they rely on ordering between them? there is useful information there
< jnewbery>
is there? bitcoind receives a transaction and sends (1) a notification of the txid; and (2) a notification of the raw tx. What is the useful information in that order? Seems arbitrary to me
< BlueMatt>
no, it also sends transactions that were either included or removed in a block connection/disconnection
< BlueMatt>
in the same tx notification that it uses for transactions added to mempool
< BlueMatt>
(none of this is documented, btw.....)
< jnewbery>
right, so if I receive a block notification and then a tx notification I can infer that the tx was received in the block... or that it was received and added to the mempool shortly after the block was connected. Again, not much useful information there
< BlueMatt>
no, brfore
< BlueMatt>
because the ordering is consistent you know that it was added after the block
< BlueMatt>
so you know that it was either part of the block or added before
< BlueMatt>
specifically, when you get a block notification, you know that youve gotten notified for all transactionw which in blocks prior to and including the block for which you were notified
< BlueMatt>
which is a useful bit of information
< jnewbery>
ah. ok, that makes sense. Thanks
< BlueMatt>
(this is, coincidentally, the bit of information our wallet cares about :p)
< jnewbery>
ok, conclusion: we need some documentation!
< BlueMatt>
100x
< cfields>
jonasschnelli: around?
< cfields>
BlueMatt: hah! I came to nag jonasschnelli about the same thing. NODE_NETWORK implies NODE_NETWORK_LIMITED, so I agree that _LIMITED should always be set.
< cfields>
otherwise, we're introducing negative service flags, which is a bad idea imo.
< BlueMatt>
heh
< cfields>
jonasschnelli: I think the complicated address selection and "alternative services" points out another issue with the BIP. When we're making outgoing connections (assuming we're not in ibd), we don't care if the node we connect to is limited or not...
< cfields>
I think what we're missing is NODE_FULL, which says nothing about willingness to serve
< Murch>
Hey, is it still accurate that pruning nodes will not serve any blocks, or are they meanwhile serving the ones they have?
< sipa>
Murch: i believe they have always served the blocks they have, but no other node would ask blocks from them due to lack of a service bit indicating that they do
< cfields>
as implemented here, looks like we'll never request them from a limited node
< cfields>
i was working up my complaint about that :)
< cfields>
sipa: what do you think about a NODE_FULL (or so) flag which indicates that the node can validate the entire chain? That way (say in a year or two from now), we can require NODE_FULL, prefer NODE_NETWORK, and tolerate NODE_NETWORK_LIMITED ? I really don't like the outbound selection policy wonkyness that bip159 gives us.
< cfields>
(maybe that's been suggested before, but I've missed it?)
< sipa>
cfields: i've suggested that, but i'm not sure it's needed
< sipa>
cfields: if it means strictly only tip blocks, that's not something you want to connect to... what if there's a reorg
< cfields>
sipa: I'm not saying that actually running that way would be useful. I don't imagine anyone would. I'm just arguing for advertising that capability, which would almost certainly be paired with some other capability. but if you don't have it, we know we're not interested in connecting.
< cfields>
so full nodes would advertise all 3, pruned would advertise 2.
< sipa>
cfields: right, but i'm not sure there is a use for something that can only serve blocks at the tip
< sipa>
cfields: so right now, we'd have LIMITED which means tip + 288 blocks below
< sipa>
at some point there appears a need for more pruning, or less bandwidth, or whatever, and only guaranteeing tip + 10 blocks deep, so a VERY_LIMITED is added for that
< sipa>
that only used 2 bits, while you'd use 3 :)
< cfields>
yes, but now nobody connects to VERY_LIMITED for a few weeks/months, whereas they could be used as a last resort with 3 bits because we know they're full and might have ~tip
< cfields>
(not the strongest argument, i realize :)
< sipa>
my point is that there is no point having a separate bit for a feature that is not separately useful
< cfields>
i guess this achieves what I'm after as-is... in a year or two, we simply require LIMITED and prefer NETWORK
< sipa>
when we're in sync, we should treat the two as identical
< BlueMatt>
sipa: we dont need to be worried about using 1 extra bit in nServices
< BlueMatt>
we have so many
< BlueMatt>
no reason to get complicated and save the ability to re-use NODE_NETWORK_LIMITED when NODE_NETWORK is set
< cfields>
sipa: sure. It's just outbound selection i'm trying to reason through
< BlueMatt>
and we have so few things we want to set in nServices
< sipa>
BlueMatt: huh? we're not talking about that
< BlueMatt>
oh, did I miss context
< sipa>
i'm totally in favor of always setting LIMITED
< BlueMatt>
ok, ignore me
< cfields>
BlueMatt: yea, I suggested a 3rd bit for NODE_FULL
< BlueMatt>
ah
< BlueMatt>
no
< BlueMatt>
why
< BlueMatt>
thats what NODE_NETWORK means....
< cfields>
heh, see above
< BlueMatt>
how about we save it for when its useful? :P
< BlueMatt>
when someone introduces a NODE_SERVES_COMPRESSED_BLOCKS bit, we should talk about splitting NODE_NETWORK, until then.......... :p