Correct the CF fee computation

[darosior]

Based on #27.

I at first wanted wanted to refactor this code because the logic was unintuitive, but i actually think it's not reasonable at all. This proposes a new algorithm which will decrease the outputs up to a fixed amount, and fail earlier if we can't pay for the CF fees. We should never create such CF txs.

[darosior]

I would prefer to set this to something much lower, like 100 or 1_000. The refill needs to pay for the CF fees, not the fb coins!!

[JSwambo]

Vm coins are, for the majority of time, less than 10,000. For all current cf_outputs distributions Vm coins are the smallest, and I think they should be excluded from paying fees.

[darosior]

I think we should not have this hack of taking value from the fb coins if this is not negligible (like a few dozens sats)

[JSwambo]

Instead of guestimating an UNREASONABLE_VALUE_DECREASE perhaps you could check the amounts for the new outputs. It doesn't make sense to take away from coin amounts without guaranteeing the amount will remain positive and above min_fbcoin_value.

So i'd say, if deducting the amount by outputs_decrease leaves the coin with more than `min_fbcoin_value', its ok. Otherwise, don't deduct yet, try again with new remainder to deduct. Like this:

remainder_to_deduct = cf_tx_fee - remainder
while remainder_to_deduct > 0:
    none_deducted = 0
    outputs_decrease = remainder_to_deduct // num_outputs
    for i in range(len(self.fbcoins)-num_outputs, len(self.fbcoins)):
        if self.fbcoins[i]["amount"]-outputs_decrease > self.min_fbcoin_value(block_height):
            self.fbcoins[i]["amount"] -= outputs_decrease
            remainder_to_deduct -= outputs_decrease
        else:
            none_deducted += 1
    if none_deducted == num_outputs:
        raise CfError("Fee cannot be generated by deducting from new outputs")

It may still make sense to raise CfError if the outputs is greater than some arbitrary unreasonable value. But I think this approach is safer?

[darosior]

Phew, Finally! I think this is now in a pretty final state: i squashed #31 here to always compute the CF fee as we go.

[JSwambo]

nit: the second break condition is stricter than the first and will always break if the first was to break. So the first isn't necessary?

[darosior]

Right, bad reflex of working with unsigned integer. It could overflow if this was the case. Here it won't but it's no big deal either

[JSwambo]

Ack 3908ba5