lukechampine/us

Does FormContract prefer outputs that have more fund?

jkawamoto opened this issue · 4 comments

This line looks like it sorts outputs and hence I assume FormContract use outputs that have more fund:

sort.Slice(outputs, func(i, j int) bool {
return outputs[i].Value.Cmp(outputs[j].Value) > 0
})

However, I made 400 outputs with 100SC each, and run our contract former for a couple of hours. Then, there are only 200 outputs. Some outputs have only 20SC while the rest of them still have 100SC. It seems like FormContract keep using the same outputs untile running out of the fund.

I found the problem. Sorting outputs doesn't work here because the biggest output won't change until the next block is made. As a result, FormContract keeps using the same output.

Hmm, I guess this is kinda subtle.

In #68 I changed the behavior of fundSiacoins to prefer non-limbo outputs. The idea was that this would reduce the likelihood of using unconfirmed outputs in contract transactions. However, it also had another effect: it increased the likelihood of double-spends, because now the wallet was not filtering out outputs that were spent in limbo transactions.

In short:

  • The problem with using UnspentOutputs(true) is that it may contain outputs created in limbo transactions.
  • The problem with using UnspentOutputs(false) is that it may contain outputs spent in limbo transactions.

I'll have to think for a bit about how best to handle this. I think it will look something like this:

  • Fetch all of the wallet's outputs, including those created in limbo transactions
  • First consider the outputs that are confirmed and have not been spent in limbo transactions. If there are enough such outputs to fund the contract, use them.
  • Then, fallback to using unconfirmed outputs created in limbo transactions, using as few as possible.

This should achieve the desired behavior, which is to prefer outputs that are known to be spendable, only resorting to unconfirmed outputs as necessary.

After #68, this fundSiacoins

outputs, err := w.UnspentOutputs(false)
if err != nil {
return nil, err
}
var balance types.Currency
for _, o := range outputs {
balance = balance.Add(o.Value)
}
if balance.Cmp(amount) < 0 {
// insufficient funds; proceed with limbo outputs
outputs, err = w.UnspentOutputs(true)
if err != nil {
return nil, err
}
}

looks like it does

First consider the outputs that are confirmed and have not been spent in limbo transactions. If there are enough such outputs to fund the contract, use them.
Then, fallback to using unconfirmed outputs created in limbo transactions, using as few as possible.

doesn't it? If I'm not mistaken, UnspentOutputs(false) returns outputs that are confirmed and have not been spent in limbo transactions.

Unfortunately no; UnspentOutputs(false) does not take Limbo transactions into account at all, even to remove spent outputs. Perhaps it should? I'm not sure.