bugfix: pass along toJSON args

[tynes]

Without passing along the arguments here, this.getJSON will default will return incorrect information.

  getJSON(network, coin) {
    network = Network.get(network);

    let addr;
    if (!coin) {
      addr = this.getAddress();
      if (addr)
        addr = addr.toString(network);
    }

    return {
      prevout: this.prevout.toJSON(),
      script: this.script.toJSON(),
      witness: this.witness.toJSON(),
      sequence: this.sequence,
      address: addr,
      coin: coin ? coin.getJSON(network, true) : undefined
    };
  }

The network will default to main, which will result in an incorrect address

[codecov-io]

Codecov Report

Merging #586 into master will not change coverage.
The diff coverage is 0%.

@@           Coverage Diff           @@
##           master     #586   +/-   ##
=======================================
  Coverage   52.82%   52.82%           
=======================================
  Files         104      104           
  Lines       27661    27661           
  Branches     4736     4736           
=======================================
  Hits        14611    14611           
  Misses      13050    13050

Impacted Files	Coverage Δ
lib/primitives/input.js	83.04% <0%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7178d9b...615b9dd. Read the comment docs.

[braydonf]

I noticed a similar issue in an RPC method so there could be more. Here was the fix: braydonf@a72c50a

Edit: added a pr for it at #587

[tynes]

Seems like we could use a test suite that tries each network and asserts for anything that is network specific

[tuxcanfly]

Nice find. Agree on need to test all places where network is not being passed.

    let addr;
    if (!coin) {
      addr = this.getAddress();
      if (addr)
        addr = addr.toString(network);
    }

in this bit, even if coin is passed, the address is always null. Looks like getAddress accepts coin. Wouldn't we want this.getAddress(coin) here, if coin is available?

[braydonf]

Anything remaining for this? A test?

[nodech]

minor: update jsdoc 👍

[tynes]

Bump: Noticing this in hsd as well, its on my TODOs to add some tests for this and get them into hsd as well. I think it will help developers a lot when they log their primitives to see the correctly formatted addresses

[braydonf]

So most transaction and block related primitives do not pass these arguments for the toJSON method, including:

• Output
• Input
• TX
• TXMeta
• MTX
• Block
• MerkleBlock

For consistency it would be good to have the similar behavior between these. I suspect that the reason for the lack of arguments for these methods is that it's typically used via JSON.stringify(tx), where there are no arguments.

What's an example of how the input is being logged?

[braydonf]

Okay, closing as toJSON is specific to JSON.stringify and doesn't include arguments. There would need to be a network property in address, and potentially others, to resolve that case (similar to #720), if the network is not set in the global correctly.