simibc: improves package documentation for simibc

tests/difference/core/driver/core_test.go

@@ -190,7 +190,7 @@ func (s *CoreSuite) consumerSlash(val sdk.ConsAddress, h int64, isDowntime bool)
 		if e.Type == channeltypes.EventTypeSendPacket {
 			packet, err := ibctestingcore.ReconstructPacketFromEvent(e)
 			s.Require().NoError(err)
-			s.simibc.Link.AddPacket(s.chainID(C), packet)
+			s.simibc.Outboxes.AddPacket(s.chainID(C), packet)
 		}
 	}
 }

tests/difference/core/driver/setup_test.go

@@ -133,8 +133,8 @@ func (s *CoreSuite) TestAssumptionsSetup() {
 	s.Require().Equal(s.offsetHeight, s.height(C)-1)
 
 	// Network is empty
-	s.Require().Empty(s.simibc.Link.OutboxPackets[P])
-	s.Require().Empty(s.simibc.Link.OutboxPackets[C])
-	s.Require().Empty(s.simibc.Link.OutboxAcks[P])
-	s.Require().Empty(s.simibc.Link.OutboxAcks[C])
+	s.Require().Empty(s.simibc.Outboxes.OutboxPackets[P])
+	s.Require().Empty(s.simibc.Outboxes.OutboxPackets[C])
+	s.Require().Empty(s.simibc.Outboxes.OutboxAcks[P])
+	s.Require().Empty(s.simibc.Outboxes.OutboxAcks[C])
 }

tests/difference/core/model/src/traceUtil.ts

@@ -32,7 +32,7 @@ const meta = {
   // Commit of interchain-security/ that the trace was generated.
   commit: childProcess.execSync('git rev-parse HEAD').toString().trim(),
   // Diff between the working tree and the commit.
-  diff: childProcess.execSync('git diff').toString().trim(),
+  // diff: childProcess.execSync('git diff').toString().trim(),
 };
 
 /**

testutil/simibc/README.md

@@ -0,0 +1,19 @@
+# simibc
+
+## What is this?
+
+A collection of utilities based on [ibc-go/testing](https://github.com/cosmos/ibc-go/tree/main/testing) which make it easier to write test scenarios involving precise orderings of
+
+- BeginBlock, EndBlock on each IBC connected chain
+- Packet delivery
+- Updating the client
+
+## Why is this useful?
+
+It is very hard to reason about tests written using vanilla [ibc-go/testing](https://github.com/cosmos/ibc-go/tree/main/testing) because the methods included in that library have many side effects. For example, that library has a notion of global time, so calling EndBlock on one chain will influence the future block times of another chain. As another example, sending a packet from chain A to B will automatically progress the block height on chain A. These behaviors make it very hard to understand, especially if your applications have business logic in BeginBlock or EndBlock.
+
+The utilities in simibc do not have any side effects, making it very easy to understand what is happening. It also makes it very easy to write data driven tests (like table tests, model based tests or property based tests).
+
+## How do I use this?
+
+Please see the function docstrings to get an idea of how you could use this package. This README is intentionally short because it is easier to maintain code and docstrings instead of markdown.

testutil/simibc/chain_util.go

@@ -11,9 +11,11 @@ import (
 	tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
 )
 
-// BeginBlock updates the current header and calls the app.BeginBlock() method.
+// BeginBlock updates the current header and calls the app.BeginBlock method.
 // The new block height is the previous block height + 1.
 // The new block time is the previous block time + dt.
+//
+// NOTE: this method may be used independently of the rest of simibc.
 func BeginBlock(c *ibctesting.TestChain, dt time.Duration) {
 
 	c.CurrentHeader = tmproto.Header{
@@ -28,7 +30,14 @@ func BeginBlock(c *ibctesting.TestChain, dt time.Duration) {
 	_ = c.App.BeginBlock(abci.RequestBeginBlock{Header: c.CurrentHeader})
 }
 
-// EndBlock and calls the preCommitCallback before the app.Commit() is called.
+// EndBlock calls app.EndBlock and executes preCommitCallback BEFORE calling app.Commit
+// The callback is useful for testing purposes to execute arbitrary code before the
+// chain sdk context is cleared in .Commit().
+// For example, app.EndBlock may lead to a new state, which you would like to query
+// to check that it is correct. However, the sdk context is cleared after .Commit(),
+// so you can query the state inside the callback.
+//
+// NOTE: this method may be used independently of the rest of simibc.
 func EndBlock(c *ibctesting.TestChain, preCommitCallback func()) (*ibctmtypes.Header, []channeltypes.Packet) {
 	ebRes := c.App.EndBlock(abci.RequestEndBlock{Height: c.CurrentHeader.Height})
 

testutil/simibc/doc.go

@@ -0,0 +1,4 @@
+/*
+simibc is a collection of utilities wrapping the ibc-go testing framework which make is easier to write test scenarios involving precise orders of packet and ack delivery and calls to BeginBlock and EndBlock.
+*/
+package simibc

testutil/simibc/ordered_outbox.go

@@ -0,0 +1,114 @@
+package simibc
+
+import channeltypes "github.com/cosmos/ibc-go/v4/modules/core/04-channel/types"
+
+// Ack represents a (sent) ack committed to block state
+type Ack struct {
+	Ack []byte
+	// The packet to which this ack is a response
+	Packet channeltypes.Packet
+	// The number of App.Commits that have occurred since this ack was sent
+	// For example, if the ack was sent at height h, and the blockchain
+	// has headers ..., h, h+1, h+2 then Commits = 3
+	Commits int
+}
+
+// Packet represents a (sent) packet committed to block state
+type Packet struct {
+	Packet channeltypes.Packet
+	// The number of App.Commits that have occurred since this packet was sent
+	// For example, if the ack was sent at height h, and the blockchain
+	// has headers ..., h, h+1, h+2 then Commits = 3
+	Commits int
+}
+
+// OrderedOutbox is a collection of ORDERED packets and acks that have been sent
+// by different chains, but have not yet been delivered to their target.
+// The methods take care of bookkeeping, making it easier to simulate
+// a real relayed IBC connection.
+//
+// Each sent packet or ack can be added here. When a sufficient number of
+// block commits have followed each sent packet or ack, they can be consumed:
+// delivered to their target. Since the sequences are ordered, this is useful
+// for testing ORDERED ibc channels.
+//
+// NOTE: OrderedOutbox MAY be used independently of the rest of simibc.
+type OrderedOutbox struct {
+	// An ordered sequence of packets from each sender
+	OutboxPackets map[string][]Packet
+	// An ordered sequence of acks from each sender
+	OutboxAcks map[string][]Ack
+}
+
+// MakeOrderedOutbox creates a new empty OrderedOutbox.
+func MakeOrderedOutbox() OrderedOutbox {
+	return OrderedOutbox{
+		OutboxPackets: map[string][]Packet{},
+		OutboxAcks:    map[string][]Ack{},
+	}
+}
+
+// AddPacket adds an outbound packet from the sender.
+func (n OrderedOutbox) AddPacket(sender string, packet channeltypes.Packet) {
+	n.OutboxPackets[sender] = append(n.OutboxPackets[sender], Packet{packet, 0})
+}
+
+// AddAck adds an outbound ack from the sender. The ack is a response to the packet.
+func (n OrderedOutbox) AddAck(sender string, ack []byte, packet channeltypes.Packet) {
+	n.OutboxAcks[sender] = append(n.OutboxAcks[sender], Ack{ack, packet, 0})
+}
+
+// ConsumePackets returns the first num packets with 2 or more commits. Returned
+// packets are removed from the outbox and will not be returned again (consumed).
+func (n OrderedOutbox) ConsumePackets(sender string, num int) []Packet {
+	ret := []Packet{}
+	sz := len(n.OutboxPackets[sender])
+	if sz < num {
+		num = sz
+	}
+	for _, p := range n.OutboxPackets[sender][:num] {
+		if 1 < p.Commits {
+			ret = append(ret, p)
+		} else {
+			break
+		}
+	}
+	n.OutboxPackets[sender] = n.OutboxPackets[sender][len(ret):]
+	return ret
+}
+
+// ConsumerAcks returns the first num packets with 2 or more commits. Returned
+// acks are removed from the outbox and will not be returned again (consumed).
+func (n OrderedOutbox) ConsumeAcks(sender string, num int) []Ack {
+	ret := []Ack{}
+	sz := len(n.OutboxAcks[sender])
+	if sz < num {
+		num = sz
+	}
+	for _, a := range n.OutboxAcks[sender][:num] {
+		if 1 < a.Commits {
+			ret = append(ret, a)
+		} else {
+			break
+		}
+	}
+	n.OutboxAcks[sender] = n.OutboxAcks[sender][len(ret):]
+	return ret
+}
+
+// Commit marks a block commit, increasing the commit count for all
+// packets and acks in the sender's outbox.
+// When a packet or ack has 2 or more commits, it is available for
+// delivery to the counterparty chain.
+// Note that 2 commits are necessary instead of 1:
+//   - 1st commit is necessary for the packet to included in the block
+//   - 2nd commit is necessary because in practice the ibc light client
+//     needs to have block h + 1 to be able to verify the packet in block h.
+func (n OrderedOutbox) Commit(sender string) {
+	for i := range n.OutboxPackets[sender] {
+		n.OutboxPackets[sender][i].Commits += 1
+	}
+	for i := range n.OutboxAcks[sender] {
+		n.OutboxAcks[sender][i].Commits += 1
+	}
+}

testutil/simibc/relay_util.go

@@ -13,11 +13,16 @@ import (
 	tmtypes "github.com/tendermint/tendermint/types"
 )
 
-// UpdateReceiverClient is used to send a header to the receiving endpoint and update
-// the client of the respective chain.
+// UpdateReceiverClient DELIVERs a header to the receiving endpoint
+// and update the respective client of the receiving chain.
+//
+// The header is a header of the sender chain. The receiver chain
+// must have a client of the sender chain that it can update.
+//
+// NOTE: this function MAY be used independently of the rest of simibc.
 func UpdateReceiverClient(sender *ibctesting.Endpoint, receiver *ibctesting.Endpoint, header *ibctmtypes.Header) (err error) {
 
-	header, err = constructTMHeader(receiver.Chain, header, sender.Chain, receiver.ClientID, clienttypes.ZeroHeight())
+	err = augmentHeader(sender.Chain, receiver.Chain, receiver.ClientID, header)
 
 	if err != nil {
 		return err
@@ -55,7 +60,12 @@ func UpdateReceiverClient(sender *ibctesting.Endpoint, receiver *ibctesting.Endp
 	return nil
 }
 
-// Try to receive a packet on receiver. Returns ack.
+// TryRecvPacket will try once to DELIVER a packet from sender to receiver. If successful,
+// it will return the acknowledgement bytes.
+//
+// The packet must be sent from the sender chain to the receiver chain, and the
+// receiver chain must have a client for the sender chain which has been updated
+// to a recent height of the sender chain so that it can verify the packet.
 func TryRecvPacket(sender *ibctesting.Endpoint, receiver *ibctesting.Endpoint, packet channeltypes.Packet) (ack []byte, err error) {
 	packetKey := host.PacketCommitmentKey(packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence())
 	proof, proofHeight := sender.Chain.QueryProof(packetKey)
@@ -93,7 +103,12 @@ func TryRecvPacket(sender *ibctesting.Endpoint, receiver *ibctesting.Endpoint, p
 	return ack, nil
 }
 
-// Try to receive an ack on receiver.
+// TryRecvAck will try once to DELIVER an ack from sender to receiver.
+//
+// The ack must have been sent from the sender to the receiver, in response
+// to packet which was previously delivered from the receiver to the sender.
+// The receiver chain must have a client for the sender chain which has been
+// updated to a recent height of the sender chain so that it can verify the packet.
 func TryRecvAck(sender *ibctesting.Endpoint, receiver *ibctesting.Endpoint, packet channeltypes.Packet, ack []byte) (err error) {
 	p := packet
 	packetKey := host.PacketAcknowledgementKey(p.GetDestPort(), p.GetDestChannel(), p.GetSequence())
@@ -126,41 +141,39 @@ func TryRecvAck(sender *ibctesting.Endpoint, receiver *ibctesting.Endpoint, pack
 	return nil
 }
 
-// constructTMHeader will augment a valid 07-tendermint Header with data needed to update
-// light client.
-func constructTMHeader(chain *ibctesting.TestChain, header *ibctmtypes.Header, counterparty *ibctesting.TestChain, clientID string, trustedHeight clienttypes.Height) (*ibctmtypes.Header, error) {
-	// Relayer must query for LatestHeight on client to get TrustedHeight if the trusted height is not set
-	if trustedHeight.IsZero() {
-		trustedHeight = chain.GetClientState(clientID).GetLatestHeight().(clienttypes.Height)
-	}
+// augmentHeader is a helper that augments the header with the height and validators that are most recently trusted
+// by the receiver chain. If there is an error, the header will not be modified.
+func augmentHeader(sender *ibctesting.TestChain, receiver *ibctesting.TestChain, clientID string, header *ibctmtypes.Header) error {
+
+	trustedHeight := receiver.GetClientState(clientID).GetLatestHeight().(clienttypes.Height)
+
 	var (
 		tmTrustedVals *tmtypes.ValidatorSet
 		ok            bool
 	)
 	// Once we get TrustedHeight from client, we must query the validators from the counterparty chain
 	// If the LatestHeight == LastHeader.Height, then TrustedValidators are current validators
 	// If LatestHeight < LastHeader.Height, we can query the historical validator set from HistoricalInfo
-	if trustedHeight == counterparty.LastHeader.GetHeight() {
-		tmTrustedVals = counterparty.Vals
+	if trustedHeight == sender.LastHeader.GetHeight() {
+		tmTrustedVals = sender.Vals
 	} else {
 		// NOTE: We need to get validators from counterparty at height: trustedHeight+1
 		// since the last trusted validators for a header at height h
 		// is the NextValidators at h+1 committed to in header h by
 		// NextValidatorsHash
-		tmTrustedVals, ok = counterparty.GetValsAtHeight(int64(trustedHeight.RevisionHeight + 1))
+		tmTrustedVals, ok = sender.GetValsAtHeight(int64(trustedHeight.RevisionHeight + 1))
 		if !ok {
-			return nil, sdkerrors.Wrapf(ibctmtypes.ErrInvalidHeaderHeight, "could not retrieve trusted validators at trustedHeight: %d", trustedHeight)
+			return sdkerrors.Wrapf(ibctmtypes.ErrInvalidHeaderHeight, "could not retrieve trusted validators at trustedHeight: %d", trustedHeight)
 		}
 	}
-	// inject trusted fields into last header
-	// for now assume revision number is 0
-	header.TrustedHeight = trustedHeight
-
 	trustedVals, err := tmTrustedVals.ToProto()
 	if err != nil {
-		return nil, err
+		return err
 	}
+	// inject trusted fields into last header
+	// for now assume revision number is 0
+	header.TrustedHeight = trustedHeight
 	header.TrustedValidators = trustedVals
 
-	return header, nil
+	return nil
 }