bitcoind-rpc icon indicating copy to clipboard operation
bitcoind-rpc copied to clipboard

Batch interface suggestion

Open fanatid opened this issue 9 years ago • 4 comments

Now batch call look like:

rpc.batch(function() { /* add some request to rpc */ }, function() { /* request callback */ })

I think it's not good, why I must write yet one function for filling rpc.batchedCalls?

And what really important, what happened if we get an exception in first function? When we try make new request (getInfo for example), batchedCalls not be null and rpc instance will be considered that we fill batch request... so, it's broke all rpc instance!

I suggest replaced batch request by something like this:

rpc.createBatch() // create copy of rpc instance and set batchedCalls as empty array
  .getRawTransaction(txid1) // put request to batchedCalls
  .getRawTransaction(txid2) // put request to batchedCalls
  .call(function() { /* request callback */ }) // make request

I'm not sure about createBatch, call and hope somebody offer more suitable names.

fanatid avatar Mar 29 '15 18:03 fanatid

I agree that the interface can be improved, and I like your suggestion. I'd personally prefer:

rpc.batch() 
  .getRawTransaction(txid1)
  .getRawTransaction(txid2)
  .call(function() { /* request callback */ })

or a more promise-oriented version:

rpc.batch() 
  .getRawTransaction(txid1)
  .getRawTransaction(txid2)
  .call()
  .then(function() {
    console.log('success!');
  })
  .catch(function(error) {
    console.log('Batch call errored:', error);
  });

do you want to try and implement this in a PR (either of the versions)?

maraoz avatar Mar 30 '15 19:03 maraoz

The batch API for levelup I think makes a fair amount of sense:

  • https://github.com/rvagg/node-levelup#batch
  • https://github.com/rvagg/node-levelup#batch_chained

braydonf avatar Mar 30 '15 19:03 braydonf

Promise not good idea here, because except batch bitcoind-rpc have other many methods.. and we should stick one style: callbacks everywhere or promise everywhere. Bluebird have cool function promisifyAll, so I think choice towards to callbacks is obvious. Thanks @braydonf clear must be added to batch (as in levelup)

fanatid avatar Mar 30 '15 19:03 fanatid

FYI, I wrote my own bitcoind rpc driver with batch behavior described here, you can see code here

fanatid avatar Aug 22 '15 15:08 fanatid