Localhost Verification in 04-channel

[AdityaSripal]

Go through all the places in 04-channel that does proof verification (these are in packet verification and channel handshakes)

• RecvPacket
• AcknowledgePacket
• TimeoutPacket (ORDERED and UNORDERED)
• TimeoutOnClose (ORDERED and UNORDERED)
• ChanOpenTry
• ChanOpenAck
• ChanOpenConfirm
• ChanCloseConfirm

In each place:

Replace connection.Verify... with the following:

func ChannelMethod() {
   // logic prior to proof verification
   // expectedVal is constructed here

    // the if statement is new logic
    // the else statement will house the old existing logic
    if channel.ConnectionHops[0] == "localhost" {
        // get the desired state directly from this chain's channelKeeper
        // this will be a specific Get function dependent on the specific method this is
        actualVal := k.Get(ICS24Path())
        if expectedVal != actual {
             return error
        }
    } else {
        // do counterparty proof verification with underlying client
        
        connection := k.GetConnection(channel.ConnectionHops[0]

        // this is a specific verify function depending on what method this is
        connection.VerifyChannelState(proof, expectedKey, expectedVal)
    }

   // continue existing logic
}

Here is the example for RecvPacket:

// example of channel function that does
// verification in special localhost case
func (k Keeper) RecvPacket(
	ctx sdk.Context,
	chanCap *capabilitytypes.Capability,
	packet exported.PacketI,
	proof []byte,
	proofHeight exported.Height,
) error {
    // ... recv packet logic

    commitment := types.CommitPacket(k.cdc, packet)

    // packet commitment verification
    if channel.ConnectionHops[0] == "localhost" {
        storedCommit, ok := k.GetPacketCommitment(ctx, packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence())
        if !ok || storedCommit != commitment {
            return error
        }
    } else {
        // do counterparty proof verification with underlying client

        // move GetConnection code down into the else block since it will error on "localhost"
        // because the "localhost" connection doesn't actually exist in-state

        // Connection must be OPEN to receive a packet. It is possible for connection to not yet be open if packet was
	// sent optimistically before connection and channel handshake completed. However, to receive a packet,
	// connection and channel must both be open
	connectionEnd, found := k.connectionKeeper.GetConnection(ctx, channel.ConnectionHops[0])
	if !found {
		return sdkerrors.Wrap(connectiontypes.ErrConnectionNotFound, channel.ConnectionHops[0])
	}

	if connectionEnd.GetState() != int32(connectiontypes.OPEN) {
		return sdkerrors.Wrapf(
			connectiontypes.ErrInvalidConnectionState,
			"connection state is not OPEN (got %s)", connectiontypes.State(connectionEnd.GetState()).String(),
		)
	}
        
        // verify that the counterparty did commit to sending this packet
	if err := k.connectionKeeper.VerifyPacketCommitment(
		ctx, connectionEnd, proofHeight, proof,
		packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence(),
		commitment,
	); err != nil {
		return sdkerrors.Wrap(err, "couldn't verify counterparty packet commitment")
	}
    }
    
    // ... continued recv packet logic
}

[jtieri]

Hey @AdityaSripal, I started working through the 09-localhost issues a bit ago and wanted to run the first pass by you to ensure everything looks correct before I complete the rest of it.

https://github.com/strangelove-ventures/ibc-go/commit/d8ab0aa81b28bfc98d4c1649d92408a90095a228

Thanks for your time on all of this!!

[crodriguezvega]

@jtieri so then you're working on this? I will assign the issue to you then. Thanks a lot!

[jtieri]

@jtieri so then you're working on this? I will assign the issue to you then. Thanks a lot!

Yepp! I'm going to take on all the issues labeled 09-localhost. Aditya expressed it may be better to complete the issues in our forked repo first so Agoric can utilize these changes sooner than later, and then sometime in your guys next sprint we can work on upstreaming these changes. (or at least that is the understanding that I walked away with)

[damiannolan]

closing in favour of #3034