Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix logic to handle multiple outputs in ExplorerWallet.buildTXHex() #26

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

daviortega
Copy link
Member

Motivation

Allow ExplorerWallet.sendTx() to handle multiple outputs

What did I do?

  1. I removed a logic to manually handle outputs and change.
    It seemed unnecessary since the bitcoinjs-lib appears to handle messy outputs just fine.

  2. I removed an Error thrown if among the outputs from buildInputsAndOutputs() an output had no address.
    It seems that buildInputsAndOutputs will always pass the "change" output without an address attribute.
    This was fixed for transactions with only one output by the logic I removed above but it fails with multiple outputs.
    Instead, I add the p2pkh address to the "change" and removed the error.

  3. I added tests for multiple outputs transaction in the integration test suite.
    I made sure previous tests that seem to use this logic still passes.

  4. I also ran npm run code-format which made the other change in the code that is unrelated to this PR - but it makes the npm test pass now.

@daviortega daviortega added the bug Something isn't working label Jul 28, 2020
@daviortega daviortega self-assigned this Jul 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant