primal100 / pybitcointools

Simple, common-sense Bitcoin-themed Python ECC library
Other
335 stars 151 forks source link

multisig error after mktx: can't concat str to bytes #46

Closed GreemDark closed 1 year ago

GreemDark commented 1 year ago
coin = Bitcoin()
...
tx = coin.mktx(inputs,outs)
for i in range(0, len(tx['ins'])):
    sig1 = coin.multisign(tx, i, script, "private_key_first")
    sig2 = coin.multisign(tx, i, script, "private_key_second")
    tx = apply_multisignatures(tx, i, script, sig1, sig2)

cryptos/transaction.py in serialize at line 182

177.        o.append(encode_1_byte(txobj["flag"]))
178    o.append(num_to_var_int(len(txobj["ins"])))
179    for inp in txobj["ins"]:
180        o.append(inp["tx_hash"][::-1])
181        o.append(encode_4_bytes(inp["tx_pos"]))
182        o.append(num_to_var_int(len(inp["script"])) + (inp["script"] if inp["script"] else bytes()))
183        o.append(encode_4_bytes(inp["sequence"]))
184    o.append(num_to_var_int(len(txobj["outs"])))
185    for out in txobj["outs"]:
186        o.append(encode_8_bytes(out["value"]))
187        o.append(num_to_var_int(len(out["script"])) + out["script"])
primal100 commented 1 year ago

Hi,

There's a test that does exactly this. I just tried it and it passes in both Python 3.8 and 3.10:

python -m unittest tests.test_coins.test_bitcoin_testnet.TestBitcoinTestnet.test_transaction_multisig

Here's an example of what a multisig transaction looks like before signing. Can you compare with what your transaction looks like? (most likely height and value are not needed in the ins, but they are there because they are returned like that from the unspent function).

{'locktime': 0, 'version': 1, 'ins': [{'height': 2377777, 'tx_hash': '191caa260dbc2bddf919fcf98d597ab9708ad5f7e7540252a06af0bcb277aaf9', 'tx_pos': 0, 'value': 1987, 'address': '2MtT7kkzRDn1kiT9GZoS1zSgh7twP145Qif', 'script': '', 'sequence': 4294967295}, {'height': 2425314, 'tx_hash': '3c0ca45ee804b43f69ff3690fc802c899db990d8218a574dbb83bc47b993773e', 'tx_pos': 0, 'value': 1979, 'address': '2MtT7kkzRDn1kiT9GZoS1zSgh7twP145Qif', 'script': '', 'sequence': 4294967295}], 'outs': [{'value': 1507, 'script': 'a91426991e5b586517a6724614823d10aff500ada4be87'}, {'value': 1959, 'script': 'a9140d37ea041956e3173831caaefc798c49ce3a6a4787'}]}

Are you providing the private keys in hex?

Stan-Polygant commented 1 year ago

Hi Paul, my name is Stan form Polygant, we develop crypto projects. You are doing great job maintaining pybitcointools. I would like to collaborate with you, please tell me how can I contact you? my telegram @polygant

GreemDark commented 1 year ago

python -v 3.8.16

python -m unittest tests.test_coins.test_bitcoin_testnet.TestBitcoinTestnet.test_transaction_multisig
Starting Bitcoin Testnet sync tests
s
----------------------------------------------------------------------
Ran 1 test in 0.000s

OK (skipped=1)

this test is skipped because we not used electrum all data prepared from btc node

primal100 commented 1 year ago

Hi @GreemDark

One of the things that makes developing and using this library very difficult is the fact that crypto data can have so many different formats. I tried my best by adding type hints and type conversion logic to help but I can only do so much. It is critical that the data entered is in the right format.

I will need more data from you to analyse this.

Two possiblities here:

You can post tx object here replacing any sensitive data with the data types (for example addresses would be replaced by str:). Also can you confirm that the private key are in hexadecimal string form? Wif format should be ok too.

Or Is it possible to re-create the issue on testnet and send me the requests you are making?

c = Bitcoin(testnet=True)
primal100 commented 1 year ago

I've identified the issue. Here's the pull request.

47

primal100 commented 1 year ago

The issue that in the original version of this library (by vbuterin) the inputs were written with an output field:

{
'output': 'txhash:txpos
}

To be more compatible with explorers and electrumx servers the default format is now:

{
'tx_hash': 'tx_hash',
'tx_pos': tx_pos
}

When the output field is provided it was causing an issue.

I still want to support the legacy format so the PR should fix this issue.

primal100 commented 1 year ago

v.2.0.6 out now on pypi please try it @GreemDark

GreemDark commented 1 year ago

@primal100 thx! now everything works fine

I'll leave it here for those who will look this issue

tx = apply_multisignatures(tx, i, script, sig1, sig2)

before (v1.36) tx = rawtransaction
after (v2.0.6) tx = dict use serialize to get rawtransaction