[RPC] Getting confirmations command #9745

34ro commented at 9:38 AM on February 12, 2017: contributor

This is aimed at saving traffic to get transaction confirmations and improve rpc throughput.

To check transaction confirmations, we needs to call "getrawtransaction" or "gettransaction". But to check confirmations of huge number of transaction, network is crowded and latency is up. "getrawtransaction" and "gettransaction" includes constancy infomation, though confirmations is just changeable field. I think it is good to split off "confirmations" from "getrawtransaction". For this, add new command "getconfirmations" to rpc command.

Add getconfirmations to rpc command a72b32a7dc

fanquake added the label RPC/REST/ZMQ on Feb 12, 2017

jonasschnelli commented at 1:53 PM on February 12, 2017: contributor

Hmm..... so this would purely be a (bandwidth) performance optimisation? Right? I haven't measured it, but I expect that most of the CPU time gets spent in GetTransaction() and the http-serving parts. I'm not sure if its worth to add another RPC call to solve a rather individual issue.

34ro commented at 2:25 PM on February 12, 2017: contributor

yes, bandwidth performance. I think it is not individual issue. Wallet users most interested in tx confirmations because it is changeable value. And we would like to detect confirmation as soon as possible to finish payment. For this reason, getrawtransaction is called so many time and network performance is down.

jonasschnelli commented at 2:57 PM on February 12, 2017: contributor

For normal use-cases, get(raw)transaction seems to be fast enough... Constantly polling is not efficient. Something that could be a better solution would be RPC long polling (https://github.com/bitcoin/bitcoin/pull/7949).

It would be possible, to keep the http-connection open (long poll) until the confirmation-count got changed (could be check on every Connect/Disconnect block). This would reduce the bandwidth dramatically.

kallewoof commented at 11:36 AM on February 13, 2017: member

A wallet would not use the RPC commands directly except if the bitcoin node is local, in which case bandwidth is not an issue. For a remote / arbitrary node, it would use some form of proxy which could have a reduced-bandwidth representation of the transactions easily enough. Without adding that representation mode to Bitcoin Core itself.

jonasschnelli commented at 11:38 AM on February 13, 2017: contributor

A wallet would not use the RPC commands directly except if the bitcoin node is local, in which case bandwidth is not an issue. For a remote / arbitrary node, it would use some form of proxy which could have a reduced-bandwidth representation of the transactions easily enough.

Ideally yes. But I could guess there are some users tunnelling RPC to a remote machine (bandwidth matters) or connecting to a node on a different machine but in the same/trusted network (bandwidth should not matter that much).

JeremyRubin commented at 12:14 PM on February 14, 2017: contributor

Hey! Just wanted to say nice work for your first PR @34ro :) In terms of code quality and formatting I think you've done a nice job & your implementation seems correct and pragmatic. Great to see that you even added tests!

Unfortunately, I agree somewhat with @jonasschnelli with regards to the necessity of this work. I don't think that the RPC bandwidth will be so large as to cause a problem in this specific case (most transactions don't have an insane amount of vins and vouts), but I do agree in general that this is worthwhile to think about (especially if you ever wanted to connect a mobile phone or something :) )

However, I do have one possible way of fixing your problem (excessive bandwidth) without needing to add a whole new RPC. You could change the order of the entries in the JSON such that the non variable-length fields are first, followed by the variable length fields. This way, your low-bandwidth client can read out just the first packet, parse far enough to read the confirmations field, and then ungracefully terminate the TCP connection. That's kind of a "dirty ugly hack" so it's only half-serious.

Another possible solution to this would be somewhat like long polling -- keep the http connection open after the RPC, and also allow a parameter fWatch to be sent. On every call to connectblock, another JSON could be sent over with just the delta (e.g., {"confirmations" : 1 } then {"confirmations": 2}). This is somewhat similar to what @jonasschnelli suggested but has the benefit of not closing the tcp socket until you're actually done with it.

laanwj commented at 12:20 PM on February 14, 2017: member

I'd say the code looks good, but yes it doesn't seem there is enough reason presented to add yet another call to the API.

And we would like to detect confirmation as soon as possible to finish payment.

The conceptual solution to this would be some kind of notification, not faster polling. @jonasschnelli has some work in introducing long-poll notifications in RPC, see #7949. That seems a more fruitful path.

laanwj commented at 1:31 PM on February 14, 2017: member

Just wanted to add that while a better notification method would be nice, practically this is already possible with -blocknotify. The number of confirmations of a transaction can only change when a block comes in. The thing I'm not sure about is whether race conditions - it may be possible that the wallet has not processed the block yet when your script launches. A -walletblocknotify that is called when the wallet processed a new block could avoid that problem.

34ro commented at 3:10 PM on February 15, 2017: contributor

Thank you for many advice. Long polling is good idea. I agree -blocknotify is practically way to detect confirmation fast. I'll wait until #7949 is merged.

in src/test/rpc_tests.cpp:None in a72b32a7dc

  48 | @@ -49,6 +49,9 @@ BOOST_AUTO_TEST_CASE(rpc_rawparams)
  49 |      BOOST_CHECK_THROW(CallRPC("getrawtransaction not_hex"), std::runtime_error);
  50 |      BOOST_CHECK_THROW(CallRPC("getrawtransaction a3b807410df0b60fcb9736768df5823938b2f838694939ba45f3c0a1bff150ed not_int"), std::runtime_error);
  51 |  
  52 | +    BOOST_CHECK_THROW(CallRPC("getconfirmations"), std::runtime_error);
  53 | +    BOOST_CHECK_THROW(CallRPC("getconfirmations not_hex"), std::runtime_error);

luke-jr commented at 7:58 PM on February 17, 2017:

Better to put these in qa/rpc-tests

in src/rpc/rawtransaction.cpp:None in a72b32a7dc

 141 | +    uint256 hash;
 142 | +    hash.SetHex(request.params[0].get_str());
 143 | +
 144 | +    CTransactionRef tx;
 145 | +    uint256 hashBlock;
 146 | +    if (!GetTransaction(hash, tx, Params().GetConsensus(), hashBlock, true))

luke-jr commented at 8:01 PM on February 17, 2017:

This requires txindex=1, otherwise only works on unconfirmed transactions.

Seems like this ought to be a wallet RPC?

laanwj commented at 3:59 PM on June 5, 2017: member

Closing this for now. It has been inactive for quite some time. Let me know if you start work on this again then I'll reopen.

laanwj closed this on Jun 5, 2017

MarcoFalke locked this on Sep 8, 2021