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

Update the QGB minimum data commitment window error message #1706

Closed
rach-id opened this issue May 5, 2023 · 0 comments · Fixed by #1713
Closed

Update the QGB minimum data commitment window error message #1706

rach-id opened this issue May 5, 2023 · 0 comments · Fixed by #1713
Assignees

Comments

@rach-id
Copy link
Member

rach-id commented May 5, 2023

          Thanks for sharing!
  1. should the 100 be extracted to a constant?
  2. I don't see why specifying a data commitment window of 99 would result in the error: "invalid average EVM block time, too short for latency limitations" because the data commitment window is not related to the average EVM block time. I would expect an error message like
const MinimumDataCommitmentWindow = 100

func validateDataCommitmentWindow(i interface{}) error {
	val, ok := i.(uint64)
	if !ok {
		return fmt.Errorf("invalid parameter type: %T", i)
	} else if val < uint64(MinimumDataCommitmentWindow) {
		return fmt.Errorf("data commitment window %v must be >= minimum data commitment window %v", val, MinimumDataCommitmentWindow)
	}
	return nil
}

Originally posted by @rootulp in #1695 (comment)

@rach-id rach-id self-assigned this May 5, 2023
@rach-id rach-id added the x/qgb label May 5, 2023
rach-id added a commit that referenced this issue May 9, 2023
…rror message (#1713)

## Overview

Closes #1706

---------

Co-authored-by: Rootul P <rootulp@gmail.com>
0xchainlover pushed a commit to celestia-org/celestia-app that referenced this issue Aug 1, 2024
…rror message (#1713)

## Overview

Closes celestiaorg/celestia-app#1706

---------

Co-authored-by: Rootul P <rootulp@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant