Fix make_change to not create half-satoshis #4836

make_change would split up inputs regardless of whether they were an odd number of satoshis and therefore would create outputs with half-satoshi amounts (or 9 decimal places in the python code). This would cause JSONRPC error: 16: bad-txns-in-belowout (perhaps both outputs are being rounded up). This fix solves that problem in that if you make-change too many times that won't happen, but I could imagine there are other places in the code that we need to be careful representing satoshis as a decimal or float number of BTC.

Ideally we wouldn't be using floats for monetary amounts at all, not in the test cases either. In Python this means either integers as satoshis or Decimal() with fixed 8 decimals.

@laanwj Seconded. This probably wants Decimal()

I remember fixing this bug, but the fix is probably lost on some long-forgotten branch.

Just using Decimal isn't good enough if I recall correctly, the default rounding mode will still give you two inputs that add up to one satoshi more than is correct.

Ah, found it:

def make_change(from_node, amount_in, amount_out, fee):
    """
    Create change output(s), return them
    """
    outputs = {}
    amount = amount_out+fee
    change = amount_in - amount
    if change > amount*2:
        # Create an extra change output to break up big inputs
        change_address = from_node.getnewaddress()
        # Split change in two, being careful of rounding:
        outputs[change_address] = Decimal(change/2).quantize(Decimal('0.00000001'), rounding=ROUND_DOWN)
        change = amount_in - amount - outputs[change_address]
    if change > 0:
        outputs[from_node.getnewaddress()] = change
    return outputs

OK, I updated the PR to use Gavin's fix instead. It fixes the test case that was breaking for me as well.

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/p4836_3a7c3483b6df12e84aefb937b85afec746b06b5e/ for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.