Multi Sig Transactions

[reedrosenbluth]

Description

This PR adds support for building Multi-Sig transactions.

Issue: #8

TODO

• [x] fix AuthField serialization bug
• [x] remove bitcoin-js dependency
• [ ] add multi-sig tx builder
• [ ] implement remaining hash modes (P2WPK-P2SH and P2WSH-P2SH)
  • can do this in a followup PR...
• [x] double check that auth clearing works as described: https://github.com/blockstack/stacks-blockchain/blob/master/sip/sip-005-blocks-and-transactions.md#transaction-signing-and-verifying
• [ ] more test vectors
• [x] merge master

Type of Change

• [x] New feature
• [ ] Bug fix
• [ ] API reference/documentation update
• [ ] Other

Does this introduce a breaking change?

List the APIs or describe the functionality that this PR breaks. Workarounds for or expected timeline for deprecation

Are documentation updates required?

Testing information

Provide context on how tests should be performed.

1. Is testing required for this change?
2. If it’s a bug fix, list steps to reproduce the bug
3. Briefly mention affected code paths
4. List other affected projects if possible
5. Things to watch out for when testing

Checklist

• [x] Code is commented where needed
• [x] Unit test coverage for new or modified code paths
• [x] npm run test passes
• [ ] Changelog is updated
• [x] Tag 1 of @yknl, @zone117x, @reedrosenbluth for review

[codecov[bot]]

Codecov Report

Merging #109 into master will increase coverage by 1.15%. The diff coverage is 86.44%.

@@            Coverage Diff             @@
##           master     #109      +/-   ##
==========================================
+ Coverage   82.76%   83.91%   +1.15%     
==========================================
  Files          28       27       -1     
  Lines        2031     2052      +21     
  Branches      438      416      -22     
==========================================
+ Hits         1681     1722      +41     
+ Misses        346      326      -20     
  Partials        4        4              

Impacted Files	Coverage Δ
src/types.ts	81.66% <60.00%> (-1.52%)	:arrow_down:
src/builders.ts	75.26% <67.74%> (-1.98%)	:arrow_down:
src/signer.ts	74.57% <73.33%> (+1.36%)	:arrow_up:
src/transaction.ts	79.43% <90.00%> (+1.34%)	:arrow_up:
src/authorization.ts	86.11% <91.27%> (+8.28%)	:arrow_up:
src/utils.ts	97.46% <94.44%> (+17.70%)	:arrow_up:
src/constants.ts	100.00% <100.00%> (ø)
src/keys.ts	90.42% <100.00%> (-0.11%)	:arrow_down:
src/index.ts

---

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 29f23dc...ff911cf. Read the comment docs.

[zone117x]

Love how clean this PR is. Code looks good. Before I approve I'd like to use this functionality in something (probably with a sidecar tx debug endpoint) for a good end-to-end test.

[yknl]

This looks good to merge 👍

[zone117x]

There's a regression with sponsored transaction signing/serialization. Here's some data I collected to help debug.

A contract-call transaction's serialized payload:

// from this branch (this is an invalid tx):
80800000000500164247d6f2b425ac5771423ae6c80c754f7172b0000000000000000100000000000030390000d357ba68874c8d2b67c497c14af2e95f726b9e21f83f624acccfca20d2f15dbf4bf81da24564a550f4e96eef6922723a8d390943996aa300eecc2123b2df9eae0031ef5ee9a226a792b93f2bfbfbc54f523eba781800000000000000000000000000000118000111ed033667fd0ad5d4136455b5d0300d16c8daf496f1ecd09697db8e62164c7247537db17e1b7aa636cd8957d4c504a01d3e63a2b7eb89ff5f7b546351e8687e030100000000021a164247d6f2b425ac5771423ae6c80c754f7172b01468656c6c6f2d776f726c642d636f6e7472616374096765742d76616c7565000000010200000000

// from master branch (this is a valid tx):
80800000000500164247d6f2b425ac5771423ae6c80c754f7172b0000000000000000100000000000030390000d357ba68874c8d2b67c497c14af2e95f726b9e21f83f624acccfca20d2f15dbf4bf81da24564a550f4e96eef6922723a8d390943996aa300eecc2123b2df9eae0031ef5ee9a226a792b93f2bfbfbc54f523eba7818000000000000000000000000000001180000a24c32196623aa72f1d53324ffbf7d203ae8b148697aaef49ff8f809cdcab5e375bb9b84d3d1a0e60073458d9a8999c5b5941c0b248a0aa66b05fcbcc6d5657f030100000000021a164247d6f2b425ac5771423ae6c80c754f7172b01468656c6c6f2d776f726c642d636f6e7472616374096765742d76616c7565000000010200000000

Also, here's the error returned by the core-node when submitting the invalid transaction:

Error: Response 400: Bad Request fetching http://127.0.0.1:20443/v2/transactions:

{
  "error": "transaction rejected",
  "reason": "SignatureValidation",
  "reason_data": {
    "message": "Signer hash does not equal hash of public key(s): 9773d279b98a0e1388875729207fe7602cda7af5 != 31ef5ee9a226a792b93f2bfbfbc54f523eba7818"
  },
  "txid": "4a407703abe75fb5d1ad489875296d9eff52ad64bbf780b44d08d412ccb5faf9"
}

[zone117x]

Draft PR in sidecar for supporting multisig transactions, currently helpful as an integration test with core-node https://github.com/blockstack/stacks-blockchain-api/pull/193

Core-node is rejecting multisig transactions with same error as the sponsored txs:

{
  "error": "transaction rejected",
  "reason": "SignatureValidation",
  "reason_data": {
    "message": "Signer hash does not equal hash of public key(s): fbcc4c8a97d1fe9d96cdf06c98ce05eccbc9ff35 != 55c50b056f6f2c500b4432b9e89828b3c8d607cf"
  },
  "txid": "54878b0cd7511af52d0ea901aa21c1f9f87c3ef254cb08bea8a82d1bb5aa4904"
}

[zone117x]

Update: the latest commit fixes multisig transactions -- they are processed by the core-node as expected.

However, the sponsored transaction regression still exists.

[reedrosenbluth]

Update: the latest commit fixes multisig transactions -- they are processed by the core-node as expected.

However, the sponsored transaction regression still exists.

I'm able to generate and submit a sponsored transaction with out any issues (on my local mocknet). @zone117x Can you sure some more detail about the transaction that isn't working for you?