stacks-network / stacks-core

The Stacks blockchain implementation
https://docs.stacks.co
GNU General Public License v3.0
3.01k stars 668 forks source link

Transactions generated can have invalid high s - force low s #180

Closed larrysalibra closed 8 years ago

larrysalibra commented 8 years ago
[
    {
        "error": "Exception: Tx hash missing from blockcypher response: {u'error': u'Error sending transaction: Transaction 749184d08770393ccb281a22275919594986e3d60aecc9dfa3dfde6662f5a429 non standard: scriptsig-high-s-signature.'}",
        "traceback": [
            "Traceback (most recent call last):",
            "  File \"/root/blockstore/blockstore/blockstored.py\", line 524, in blockstore_name_register",
            "    subsidy_public_key=public_key, user_public_key=user_public_key )",
            "  File \"/root/blockstore/blockstore/lib/operations/register.py\", line 235, in broadcast",
            "    response = serialize_sign_and_broadcast(change_inputs, outputs, private_key_obj, blockchain_broadcaster)",
            "  File \"/usr/local/lib/python2.7/dist-packages/pybitcoin/transactions/network.py\", line 156, in serialize_sign_and_broadcast",
            "    response = broadcast_transaction(signed_tx, blockchain_client)",
            "  File \"/usr/local/lib/python2.7/dist-packages/pybitcoin/transactions/network.py\", line 51, in broadcast_transaction",
            "    return blockcypher.broadcast_transaction(hex_tx, blockchain_client)",
            "  File \"/usr/local/lib/python2.7/dist-packages/pybitcoin/services/blockcypher.py\", line 89, in broadcast_transaction",
            "    raise Exception(err_str)",
            "Exception: Tx hash missing from blockcypher response: {u'error': u'Error sending transaction: Transaction 749184d08770393ccb281a22275919594986e3d60aecc9dfa3dfde6662f5a429 non standard: scriptsig-high-s-signature.'}"
        ]
    }
]

From slack:

@jcnelson [6:06 AM]: @larrysalibra: looks like it's a manifestation of this: https://bitcoin.stackexchange.com/questions/38252/the-complement-of-s-when-s-curve-order-2

can you try re-sending? I'll take a look at our ECDSA code and see if we can force low-s

https://bitcoin.stackexchange.com/questions/38252/the-complement-of-s-when-s-curve-order-2

muneeb-ali commented 8 years ago

I'm also hitting this when trying to preorder using blockcypher's API

non standard: scriptsig-high-s-signature.

Maybe @acityinohio can get some Blockcypher engineers to comment on this as well (thanks, Josh!)

acityinohio commented 8 years ago

Yup, we don't accept high-s sigs anymore: https://medium.com/blockcypher-blog/enforcing-low-s-values-to-eliminate-a-bitcoin-network-attack-3582fc0ae948

I think the ECDSA libraries we use in BlockCypher's SDKs should force low-s sigs (confirmed in Go/Python SDK, pretty sure Ruby does too) in case that helps, e.g., via btcsuite's excellent btcec package: https://github.com/btcsuite/btcd/tree/master/btcec

larrysalibra commented 8 years ago

Ran into same issue again with different address pairs.

muneeb-ali commented 8 years ago

Thanks @acityinohio for the quick update! Here is the relevant paragraph from the post

The Bitcoin Core developers acted swiftly in light of the attack, releasing a non-consensus-breaking bugfix that — by default — only relays “Low S Value”-signed transactions throughout the network. Miners can still accept higher S Value-signed transactions, but they will be harder to propagate around the network; most nodes stick to the default behavior.

Looks like we need to update how we're signing transactions.

acityinohio commented 8 years ago

Sure thing @muneeb-ali ! If you're using python, this is the commit in our SDK that moved us over to BIP66 compliance there: https://github.com/blockcypher/blockcypher-python/commit/2c568d829aa41755abebf1fef6202ec3aab91093

muneeb-ali commented 8 years ago

Posting comment from @jcnelson

regarding the high-s problem you've been encountering, try pulling the latest bitcoin package from PyPI. Version 1.1.39 should work (Blockstore currently pulls in 1.1.36). We're about to make that the new requirement, since it incorporates the BIP62 fix that forces the canonical S-value in ECDSA signatures (which Blockcypher now expects).

I'll update the requirements in the next version on PyPi

jcnelson commented 8 years ago

Turns out this has been fixed in pybitcointools (bitcoin in PyPI) for some time now. We should bump the required bitcoin version to 1.1.39.

muneeb-ali commented 8 years ago

Fixed in blockstore 0.0.9 7194dc0b01195385d4967d3d8a2f945f8ee78a24