Fix panic in handleRPCFormContract

[ChrisSchinnerl]

Ran into a panic on our CI today while fixing tests. The panic is related to handleRPCFormContract.

  go.sia.tech/coreutils/rhp/v4.(*Server).handleRPCFormContract(0xc0011dd700, {0x29c8270, 0xc00013da20})
  	/home/runner/go/pkg/mod/go.sia.tech/[email protected]/rhp/v4/server.go:615 +0x3297
  go.sia.tech/coreutils/rhp/v4.(*Server).handleHostStream(0xc0011dd700, {0x29c8270, 0xc00013da20}, 0xc0011e4180)
  	/home/runner/go/pkg/mod/go.sia.tech/[email protected]/rhp/v4/server.go:1033 +0x2a33
  go.sia.tech/coreutils/rhp/v4.(*Server).Serve.func1()
  	/home/runner/go/pkg/mod/go.sia.tech/[email protected]/rhp/v4/server.go:1103 +0xe5
  created by go.sia.tech/coreutils/rhp/v4.(*Server).Serve in goroutine 997336
  	/home/runner/go/pkg/mod/go.sia.tech/[email protected]/rhp/v4/server.go:1095 +0x126

Seems like the server doesn't sufficiently validate some data received from the client and the client is sending potentially faulty data.

[lukechampine]

My guess is it's coming from here: https://github.com/SiaFoundation/coreutils/blob/b5e84c03d17f6f0f4ea5b7e33078b1db37308d4f/rhp/v4/server.go#L586C1-L596C3

        // update renter input basis to reflect our funding basis
        if basis != req.Basis {
                hostInputs := formationTxn.SiacoinInputs[len(formationTxn.SiacoinInputs)-len(req.RenterInputs)]
                formationTxn.SiacoinInputs = formationTxn.SiacoinInputs[:len(formationTxn.SiacoinInputs)-len(req.RenterInputs)]
                txnset, err := s.chain.UpdateV2TransactionSet([]types.V2Transaction{formationTxn}, req.Basis, basis)
                if err != nil {
                        return errorBadRequest("failed to update renter inputs from %q to %q: %v", req.Basis, basis, err)
                }
                formationTxn = txnset[0]
                formationTxn.SiacoinInputs = append(formationTxn.SiacoinInputs, hostInputs)
        }

Looks like the order is mixed up: the renter inputs are first, followed by the host inputs, so it should be:

        // update renter input basis to reflect our funding basis
        if basis != req.Basis {
---             hostInputs := formationTxn.SiacoinInputs[len(formationTxn.SiacoinInputs)-len(req.RenterInputs)]
---             formationTxn.SiacoinInputs = formationTxn.SiacoinInputs[:len(formationTxn.SiacoinInputs)-len(req.RenterInputs)]
+++             hostInputs := formationTxn.SiacoinInputs[len(req.RenterInputs):]
+++             formationTxn.SiacoinInputs = formationTxn.SiacoinInputs[:len(req.RenterInputs)]
                txnset, err := s.chain.UpdateV2TransactionSet([]types.V2Transaction{formationTxn}, req.Basis, basis)
                if err != nil {
                        return errorBadRequest("failed to update renter inputs from %q to %q: %v", req.Basis, basis, err)
                }
                formationTxn = txnset[0]
                formationTxn.SiacoinInputs = append(formationTxn.SiacoinInputs, hostInputs)
        }

I'm wondering if we even need to be removing the host inputs before updating in the first place, though...?

[ChrisSchinnerl]

That was my initial feeling as well. Deployed a branch with a println within the if and it does indeed print every time we encounter the panic.