lukechampine/us

FormContract/RenewContract should check how long the unconfirmed transaction chain is

jkawamoto opened this issue · 3 comments

To my understanding, a host can remove a contract after forming it if the associated unconfirmed transaction chain gets too long. Would it be possible that it also happen if renewing a contract? In other words, will a host remove a renewed contract if the host finds the renewing transaction uses an unconfirmed transaction after it accepts the renew request?

Anyway, it'd be better to check whether the unconfirmed transaction chain is too long and if so it should decline to form/renew contracts. Otherwise, we receive a not-existing contract revision from FormContract/RenewContract.

Currently, FormContract will try to fund the contract without unconfirmed outputs first, and fall back only if there are not enough confirmed outputs. I'm not sure how much more can be done here without introducing a lot more complexity. FormContract could refuse to form contracts with unconfirmed outputs at all, but that's pretty limiting. Or it could try to only use outputs that have at most one unconfirmed parent, but in most cases that would only get you one more contract per block. (Also, the concept of "length of unconfirmed chain" doesn't currently exist anywhere in the code.) Lastly, there is always some risk that a reorg will revert a contract, so you'll always need to handle that case in order to be robust.

My recommendations are:

  • Use output splitting to ensure you always have lots of confirmed outputs.
  • Check your confirmed balance before attempting to form/renew.
  • Wait a few blocks between forming a contract and using it. (This might be less feasible for renewing.)

I think FormContract is fine so far but RenewContract would be more serious especially if hosts can remove renewed contracts. It might be worth considering to use only confirmed outputs to renew a contract.

With the contract pool contract creation should not be a problem, we can just let new contracts sit a couple of blocks as Luke mentioned and use the oldest contracts instead. But with renewing we don't have the luxury to "guess" that the contracts will renew because with so many contracts we have to manage in such a short 144 block window, we need to be 100% sure contracts are renewed successfully. So renewing with confirmed outputs only does sound like the best move. Then on our end with the metadata-server, we should give priority of renewing contracts over forming new ones @jkawamoto. I don't think we have that priority ranking yet, will open issue for it. https://github.com/storewise/s3-gateway/issues/194