Don't count change transactions in totalSpent or totalReceived.

[STRML]

This puts bitcore's calculations of spent and received in line with
the behavior of other block explorers.

Fixes #373 and bitpay/insight-api#351.

[STRML]

Also, in general, is there a reason why getAddressSummary() defines variables outside the callback from async.parallel(), rather than grabbing them from the second argument?

  var outputs;
  var inputs;

  async.parallel(
    [
      function(next) {
        self.getInputs(address, opt, function(err, ins) {
          inputs = ins;
          next(err);
        });
      },
      function(next) {
        self.getOutputs(address, opt, function(err, outs) {
          outputs = outs;
          next(err);
        });
      }
    ],
    function(err) {
      if(err) {
        return callback(err);
      }

could be shortened to

  async.parallel(
    [
      function(next) {
        self.getInputs(address, opt, next);
      },
      function(next) {
        self.getOutputs(address, opt, next);
      }
    ],
    function(err, results) {
      if(err) {
        return callback(err);
      }
      var inputs = results[0], outputs = results[1];

[STRML]

Added some new fixes for mempool transactions + corresponding tests. Also added unconfirmedSpent and unconfirmedReceived which has been useful for us to track address comings and goings without the possibility of a simultaneous spend and receive screwing up the numbers.

[braydonf]

I think we should standardize on this is some way, and clarify, especially since it's expected from other block explorers. Technically, the address is receiving the amount again, so there is unfortunate ambiguity to what "received" means (and there are others too).

[STRML]

Right. That's why I've added the fields, which has really helped in our usage.

Perhaps it makes sense to copy some of the output from a popular explorer, like Blockchain.info. But this is also something that more thorough documentation can fix.

[braydonf]

I think the best way we can handle this case is to add a value called "change", this way the values for "totalRecieved" and "totalSpent" don't need to change. And there is additional information to be able to calculate "totalRecieved" excluding change. This could also be more simply expressed as a "delta".

Also we need to know the amount of satoshis that are being spent for each input to accurately calculate the change amount (not currently available and is currently expensive to query) considering a transaction such as this:

• input 1 spending from address A with 50,000 satoshis
• input 2 spending from address B with 200,000 satoshis
• input 3 spending from address C with 1,000,000 satoshis
• output 1 receiving to address D with 25,000 satoshis
• output 2 receiving to address A with 1,225,000 satoshis

The results would be for address A:

• total change: 50,000 satoshis
• total received: 1,225,000 satoshis
• total spent: 50,000 satoshis
• delta: +1,175,000 satoshis

Being able to query the amount spent for an input more efficiently for any output would also be necessary for some other optimizations for caching large address queries. However this could deoptimize syncing times and queries for single-use addresses. There may be a better solution, but may require possible more major reworking.

[STRML]

Honestly, I think this is just more work for consumers - every other blockchain explorer API we've used discounts change transactions in the way this PR is doing it. Yeah, it'd be a major version upgrade but it is absolutely vital that it counts these transactions properly. Why would bitcore-node want to create surprise there?

[STRML]

v2 would have been a very good time to clean up the output of the Address service and fix the change outputs.

Are there not plans to do this? Failing to merge this before the rewrite will make it difficult to rebase and fix the merge errors.

[braydonf]

There currently are no plans to do this, however it's non-trivial, and I think better suited for either another service or another release.

[STRML]

It's a shame to hear that. I consider the data emitted from the API nearly useless without proper change accounting.

We've been using this patch for over a month now without issues. I am surprised to see such flippant dismissal of this work.

[STRML]

Bitcore has decidedly non-standard behavior here (different than all major block explorers), and you closed this PR without giving us a chance to rebase or talk about what needs to be done to get it in v2?

This is no way to treat contributors. Don't expect more contributions from our team.

[braydonf]

I've spawned an issue with more details here: #400

[retiredment]

Hello @STRML

Thank you for this,

Is it still okay to use this on the latest release?

I just want to run a free Blockchain API but this incorrect Total Receive is kinda killing me,
I might be able to fix it somehow, but performance-side I'm not that good yet.

I'd rather use a professional's solution instead of doing it myself and causing huge performance drop.

[STRML]

@naghajani Unfortunately this needs a total rework to work on the newest version of Bitcore.