Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor packet relaying and increase signed_blocks_window in e2e tests #967

Closed
MSalopek opened this issue May 18, 2023 · 0 comments · Fixed by #968
Closed

refactor packet relaying and increase signed_blocks_window in e2e tests #967

MSalopek opened this issue May 18, 2023 · 0 comments · Fixed by #968
Assignees
Labels
scope: testing Code review, testing, making sure the code is following the specification. type: bug Issues that need priority attention -- something isn't working type: tech-debt Slows down development in the long run

Comments

@MSalopek
Copy link
Contributor

MSalopek commented May 18, 2023

Problem

relayPackets action in e2e is non-deterministic & the signed_blocks_window is set too low causing misinterpretation of test results such as:

Closing criteria

Make relayPackets function wait for at least 1 block to be produced to ensure all Txs have been included in a block.
Make signed_blocks_window at least 10 (instead of 2) so validators have some room to breathe

Problem details

At the time of writing the relayPackets helper func in e2e testing looks like this:

func (tr TestRun) relayPackets(
	action relayPacketsAction,
	verbose bool,
) {
	// hermes clear packets ibc0 transfer channel-13
	//#nosec G204 -- Bypass linter warning for spawning subprocess with cmd arguments.
	cmd := exec.Command("docker", "exec", tr.containerConfig.instanceName, "hermes", "clear", "packets",
		"--chain", string(tr.chainConfigs[action.chain].chainId),
		"--port", action.port,
		"--channel", "channel-"+fmt.Sprint(action.channel),
	)
	if verbose {
		log.Println("relayPackets cmd:", cmd.String())
	}

	bz, err := cmd.CombinedOutput()
	if err != nil {
		log.Fatal(err, "\n", string(bz))
	}
}

Please notice that the function simply invokes hermes clear packets and does not have a way of confirming that packet send Txs were included in a block.

This can lead to weird and confusing situations where the packets are relayed, but the state is unmodified becaue not enough time had elapsed (the Tx was not included in a block, no state was modified).

This was especially confusing during Downtime tests for soft opt-out.
In this scenario we have the following steps:

  1. redelegate stake from a validator so it is in bottom 5% of validator power
  2. relay info about the redelegation
  3. initiate downtime by excluding the validator from the network

Here, step 3 would begin before results from 2 were commited to state. When a downtime was initiated, > 2/3 of validator power would be excluded from the network causing the chain to halt.

The solution
Wait a couple blocks after relaying to ensure all operations are completed.

func (tr TestRun) relayPackets(
	action relayPacketsAction,
	verbose bool,
) {
	// hermes clear packets ibc0 transfer channel-13
	//#nosec G204 -- Bypass linter warning for spawning subprocess with cmd arguments.
	cmd := exec.Command("docker", "exec", tr.containerConfig.instanceName, "hermes", "clear", "packets",
		"--chain", string(tr.chainConfigs[action.chain].chainId),
		"--port", action.port,
		"--channel", "channel-"+fmt.Sprint(action.channel),
	)
	if verbose {
		log.Println("relayPackets cmd:", cmd.String())
	}

	bz, err := cmd.CombinedOutput()
	if err != nil {
		log.Fatal(err, "\n", string(bz))
	}

	tr.waitBlocks(action.chain, 1, 10*time.Second)   // wait for block inclusion
}
@MSalopek MSalopek added type: bug Issues that need priority attention -- something isn't working scope: testing Code review, testing, making sure the code is following the specification. type: tech-debt Slows down development in the long run labels May 18, 2023
@MSalopek MSalopek self-assigned this May 18, 2023
@MSalopek MSalopek changed the title refactor packet relaying in e2e tests refactor packet relaying and increase signed_blocks_window in e2e tests May 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope: testing Code review, testing, making sure the code is following the specification. type: bug Issues that need priority attention -- something isn't working type: tech-debt Slows down development in the long run
Projects
Status: ✅ Done
Development

Successfully merging a pull request may close this issue.

1 participant