JSON-RPC: add filter{clearall,clear,add}, a block and tx notification system #1386

jgarzik commented at 3:39 AM on May 24, 2012: contributor

These RPCs manipulate a set of named lists, each of which matches zero or more bitcoin addresses.

If a match is found in the TX memory pool, the command specified by arg -filtertx (or -filter, if missing) is executed.

If a match is found in a newly [re-]accepted block (SetBestChain), the command specified by arg -filterblock (or -filter, if missing) is executed.

The following command string substituations are performed, before the notification command is executed:

    %t      "tx" or "block"
    %h      hex string of 256-bit block hash or tx hash

in src/filter.cpp:None in 55e692a17b outdated

  17 | +static string strNotifyTx;
  18 | +static string strNotifyBlock;
  19 | +
  20 | +Value filterclearall(const Array& params, bool fHelp)
  21 | +{
  22 | +    if (fHelp || params.size() > 1)

roques commented at 1:24 PM on May 24, 2012:

Shouldn't this be params.size() > 0?

in src/filter.h:None in 55e692a17b outdated

  13 | +class CBlock;
  14 | +
  15 | +class CFilter
  16 | +{
  17 | +private:
  18 | +    std::string name;

roques commented at 1:49 PM on May 24, 2012:

As far as I can see CFilter.name is currently unused. It should be enough to have the name as index of CBloomers.map for now.

luke-jr commented at 3:13 AM on May 26, 2012: member

The code itself (and JSON-RPC help docs) could use better documentation, IMO. How about letting each filter have its own command?

roques commented at 10:39 PM on May 26, 2012: contributor

I really could use this functionality right now.

While you still can, how about renaming the RPC-commands and adding a related one? notifystart <name> <bitcoinaddress>|0x<hexstring> Setup notifications named name about transactions containing bitcoinaddress or hexstring notifystop [<name> [<bitcoinaddress>|0x<hexstring>]] Stop notifications under name (or all) about bitcoinaddress (or all) listnotifications [<name>] List notifications setup under name or all notifications if name is not given.

When the notification command is run %n could be replaced with the name under which the notification was setup. If a transaction matched several filters and %n is present in the command, command should be called for each matching filter.

To prevent people with RPC-access to gain shell access as the user running bitcoind, Everything except [a-zA-Z0-9_] should be filtered from name. Properly shell-quoting name could suffice in theory, but writing secure shell scripts is hard and in my opinion the restriction is worth the extra ease of security.

Please excuse me for not substantiating this feature proposal with code.

in src/filter.cpp:None in 102b1be44d outdated

 135 | +    boost::thread t(runCommand, strCmd); // thread runs free
 136 | +}
 137 | +
 138 | +void FilterSetup()
 139 | +{
 140 | +    string strNotifyTx = GetArg("-filtertx", "");

gavinandresen commented at 2:48 PM on June 1, 2012:

local variables strNotifyTx/Block are overriding the static strNotifyTx/Block here?

jgarzik commented at 6:59 PM on June 8, 2012:

yes, that is a bug

in src/filter.cpp:None in 102b1be44d outdated

  30 | +    return true;
  31 | +}
  32 | +
  33 | +Value filterclear(const Array& params, bool fHelp)
  34 | +{
  35 | +    if (fHelp || params.size() != 1)

gavinandresen commented at 2:51 PM on June 1, 2012:

Maybe filterclear '*' instead of separate filterclearall ?

jgarzik commented at 4:30 PM on June 8, 2012: contributor

Copying and expanding on the comments in @gavinandresen's gist RPC interface document https://gist.github.com/2839617

'name' should be restricted to [a-zA-z0-9] etc. as suggested
although the implementation of the interface is currently exact match, the semantics are fuzzy match. users of this interface should perform their own exact matching, to prepare for an implementation that uses bloom filters or other fuzzy matching. This behavior should be documented somewhere.
I dislike the use of * because it creates minor command line PITAs... but I will go with consensus if others want '*' rather than filterclearall
I agree better docs are needed
Multi-match behavior should be addressed, as @roques hints. We could call the filter multiple times, or we could exec once with a comma-separated list. Currently it stops at first match.
Contra @gavinandresen's opinion, I continue to think sending HTTP requests over a pipelined connection is superior to continually exec'ing a program. @gavinandresen's objections to such methods -- getting SSL and auth correct is difficult -- remain valid.

luke-jr commented at 5:30 PM on June 8, 2012: member

I suggest allowing '_' in name, so PCRE \w matches.
I think '*' fits better here, but don't care strongly. Perhaps make the argument optional, so filterclear() does all?
How about supporting longpolling? It's well-established for JSON-RPC already and seems to work reasonably well. With multithreading support, it's even fairly easy to implement (see #1355)

jgarzik commented at 5:37 PM on June 8, 2012: contributor

longpolling-like behavior might indeed be a reasonable solution. That would obviate the need for an HTTP callback with its own authentication.

JSON-RPC: add filter{clearall,clear,add}, a block and tx notification system

These RPCs manipulate a set of named lists, each of which matches
zero or more bitcoin addresses.

If a match is found in the TX memory pool, the command specified by
arg -filtertx (or -filter, if missing) is executed.

If a match is found in a newly [re-]accepted block (SetBestChain),
the command specified by arg -filterblock (or -filter, if missing)
is executed.

The following command string substituations are performed, before
the notification command is executed:

    %t      "tx" or "block"
    %h      hex string of 256-bit block hash or tx hash

94d99c3e17

filter: fix bug Gavin noticed ac13b8a30a

jgarzik commented at 1:39 PM on June 13, 2012: contributor

Rebased, but it no longer builds or works after the CTxDestination updates in #master

luke-jr commented at 12:07 AM on June 19, 2012: member

@jgarzik Do you plan to finish rebasing it?

jgarzik commented at 3:57 PM on June 27, 2012: contributor

Closing - out of date. Consensus seems to agree with the approach, so the pull request will be reopened if/when the branch is updated (volunteers welcome). Presumably, this would coincide with the introduction of the discussion P2P protocol bloom filter changes.

jgarzik closed this on Jun 27, 2012

DrahtBot locked this on Sep 8, 2021