Eliminate nonsense logic during contract negotiation

[huetsch]

This one left me scratching my head for a couple days.

The 3rd field returned by TransactionBuilder.ViewAdded() is a slice of SiafundInput indices.

When the host sets up the collateral transaction, the code was taking this (empty) list of SiafundInputs indices and treating them as SiacoinOutput indices. The host was then gathering up an empty slice of SiacoinOutputs based on these indices and sending it back to the renter, which was trying to put them into the final transaction to be submitted to the blockchain with the FileContract.

None of it really makes any sense to me, as there shouldn't be any SiacoinOutputs in that FileContract transaction.

The code was harmless in execution but it should be eliminated from the codebase because it is very confusing.

[DavidVorick]

I believe that logic is there to enable the same thing that you just changed in SendSiacoins. With this logic in the code, we can change the renter and host so that they do not need to create setup transactions when they are forming file contracts. This would save blockchain space.

If you merge this code, any attempt to eliminate the setup transaction would break compatibility with upgraded nodes.

[huetsch]

Ahh, so the idea is that you could add refund outputs when forming a file contract. That hadn't occurred to me.

And yet, treating Siafund inputs as Siacoin outputs - your ViewAdded() return value at https://github.com/NebulousLabs/Sia/pull/3150/files#diff-78abebc4c3d7837e64502e31c5742f3cL52 - is clearly incorrect.

I'll come back to this a little later and take a stab at directly setting up the optimized refund transactions in one go.