achow101 / coin-selection-simulation

13 stars 5 forks source link

change position not null, but tx has only one output #16

Open remyers opened 10 months ago

remyers commented 10 months ago

Bitcoin version: master (c80f57ba5) coin-selection-simulation: main (c604fad)

I'm getting an assert because the change position is not null, but the tx has only one output. I'm not sure how to debug this further. Any suggestions?

~/github/coin-selection-simulation$ sudo scripts/simulation.py --scenario scenarios/bustabit-2019-2020.csv ~/github/bitcoin/test/config.ini results/
2024-01-05T15:52:09.037000Z TestFramework (INFO): Initializing test directory /tmp/bitcoin_coin_sel_sim_qoh2dde6
2024-01-05T15:52:09.300000Z TestFramework (INFO): Based on branch master(c80f57ba575af96890f185765a53a62ef58ef2c8)
2024-01-05T15:52:09.300000Z TestFramework (INFO): This simulation's Unique ID: 9a9224c4d27f42b1891b27328c20a3f6
2024-01-05T15:52:09.331000Z TestFramework (INFO): Mining blocks for node0 to be able to send enough coins
2024-01-05T15:52:10.236000Z TestFramework (INFO): Simulating using scenario: bustabit-2019-2020
2024-01-05T15:52:10.236000Z TestFramework (INFO): 0 operations performed so far
Traceback (most recent call last):
  File "/home/remyers/github/coin-selection-simulation/scripts/simulation.py", line 582, in <module>
    CoinSelectionSimulation().main()
  File "/home/remyers/github/coin-selection-simulation/scripts/framework.py", line 195, in main
    self.run()
  File "/home/remyers/github/coin-selection-simulation/scripts/simulation.py", line 540, in run
    assert len(dec["tx"]["vout"]) == 2
AssertionError
[node 0] Cleaning up leftover process

I added some debugging lines and it looks like before the assert the state is:

event 2 success= 1 fee= 600 change_pos= 0

event 4 change_aps= 1 success= 1 fee= 600 change_pos= 0

len(dec["tx"]["vout"])= 1 change_pos= 0

dec["tx"]={'txid': '6ab21c3263e3c39d9b4abf1e34d40503f8e171ea5f089962ad81e7d0070c67c5', 'hash': '6ab21c3263e3c39d9b4abf1e34d40503f8e171ea5f089962ad81e7d0070c67c5', 'version': 2, 'size': 82, 'vsize': 82, 'weight': 328, 'locktime': 0, 'vin': [{'txid': '453c271c91d8f159d79c73f314069c9a8cd5aa769c591e9b42c66f8e1ff59934', 'vout': 0, 'scriptSig': {'asm': '', 'hex': ''}, 'sequence': 4294967293}], 'vout': [{'value': Decimal('0.00329000'), 'n': 0, 'scriptPubKey': {'asm': '0 727e19b73e721b43568abe335af127e957432da0', 'desc': 'addr(bcrt1qwflpnde7wgd5x452hce44uf8a9t5xtdqh2mkp9)#utql76pz', 'hex': '0014727e19b73e721b43568abe335af127e957432da0', 'address': 'bcrt1qwflpnde7wgd5x452hce44uf8a9t5xtdqh2mkp9', 'type': 'witness_v0_keyhash'}}]}

remyers commented 10 months ago

It appears that an event type 4 (aps_create_tx_internal) sets change_pos to 0 if the APS (avoid partial spends) attempt finds a solution without a change output. Should the simulation be run without APS? otherwise it seems like 0 is a valid change position result for a aps_create_tx_internal

remyers commented 10 months ago

Actually it seems like when event type 2 ( normal_create_tx_internal ) has no change output, but returns change_pos as 0 that that is triggering the assert. I'm still not sure why others haven't seen this problem. Did something changed in bitcoin?

achow101 commented 10 months ago

Ah, that looks like a bug in Bitcoin Core. The return value for change position was changed from an int to an optional, but it is still being output as an int in a few places, like here. The conversion seems to be wrong, I think that's supposed to be a -1 rather than 0.

remyers commented 10 months ago

Thanks for the confirmation! I will suggest these fixups in @murchandamus 's coingrinder PR after it is rebased. It doesn't seem worth a stand alone PR to core. In the mean time I can make the changes locally so my simulations run.

murchandamus commented 10 months ago

I’d expect the fix to an outright bug like that get merged easily, and it might be better to fix separately, since CoinGrinder doesn’t seem to be on many people’s priorities, it’s been open since July.