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

Added range checks in event parsers. Issue #180 #181

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

taco-paco
Copy link
Contributor

No description provided.

krebernisak
krebernisak previously approved these changes Dec 13, 2022
Comment on lines 53 to 59
func rangeCheck(felt *caigotypes.Felt, lowerBound *big.Int, upperBound *big.Int) error {
if !(felt.Int.Cmp(lowerBound) >= 0 && felt.Int.Cmp(upperBound) <= 0) {
return errors.New("invalid: value is out of range")
}

return nil
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: let's structure this utility as an object we can reuse:

type Range struct {
	Lower *big.Int
	Upper *big.Int
}

// Notice: we don't pass in Felt here and we keep this fn more generic
func (r *Range) Check(n *big.Int) (error) {
        if !(n.Int.Cmp(r.Lower) >= 0 && n.Int.Cmp(r.Upper) <= 0) {
		return fmt.Errorf("invalid: value %v is out of range [%v,%v]", n, r.Lower, r.Upper)
	}

	return nil
}

Which will allow for less boilerplate:

err := uint64Range.Check(eventData[observationsLenIndex])
if err != nil {
    return NewTransmissionEvent{}, err
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@krebernisak krebernisak linked an issue Dec 13, 2022 that may be closed by this pull request
@taco-paco taco-paco temporarily deployed to integration December 21, 2022 07:57 — with GitHub Actions Inactive
@taco-paco taco-paco temporarily deployed to integration December 21, 2022 08:03 — with GitHub Actions Inactive
@github-actions
Copy link

github-actions bot commented Dec 21, 2022

Smoke Test Results

1 tests  ±0   1 ✔️ ±0   6m 53s ⏱️ -9s
1 suites ±0   0 💤 ±0 
1 files   ±0   0 ±0 

Results for commit bed4f64. ± Comparison against base commit 693f776.

♻️ This comment has been updated with latest results.

@taco-paco taco-paco temporarily deployed to integration December 21, 2022 11:34 — with GitHub Actions Inactive
@taco-paco taco-paco temporarily deployed to integration December 21, 2022 11:39 — with GitHub Actions Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Handle overflows in event parsers
2 participants