Skip to content
This repository was archived by the owner on Mar 14, 2025. It is now read-only.

Do not reject entire observations based on gas or token price#930

Merged
matYang merged 6 commits intoccip-developfrom
remove-validate-obs
Jun 3, 2024
Merged

Do not reject entire observations based on gas or token price#930
matYang merged 6 commits intoccip-developfrom
remove-validate-obs

Conversation

@matYang
Copy link
Collaborator

@matYang matYang commented May 28, 2024

Motivation

Removal of static token price changes a node’s view of destTokens. The prices reported by nodes that accepted new job specs will contain fewer tokens than the prices reported by nodes that haven’t, each release will reject the observations from the other.

Solution

Do not reject entire observations based on price. Prices being valid in an observation does not increase trustworthiness of its interval.

Instead, we can reject a token if len(not_nil(obs[token])l) < f + 1.
Reasons:

  1. If len(not_nil(obs[token])) >= f + 1, then [1, f+1] honest node has included the token, therefore we know this is a legit token
  2. Knowing this is a legit token, taking the median of prices aggregated from 2f+1 obs is sufficient to guard against f faults.
  3. Validation cannot require more than f + 1 obs, that’d allow <f faulty nodes to halt price reporting for the token

@matYang matYang marked this pull request as ready for review June 2, 2024 19:29
@matYang matYang requested a review from a team as a code owner June 2, 2024 19:29
@matYang matYang merged commit 7a6f8a6 into ccip-develop Jun 3, 2024
@matYang matYang deleted the remove-validate-obs branch June 3, 2024 15:24
matYang added a commit that referenced this pull request Jun 3, 2024
## Motivation
Removal of static token price changes a node’s view of destTokens. The
prices reported by nodes that accepted new job specs will contain fewer
tokens than the prices reported by nodes that haven’t, each release will
reject the observations from the other.

## Solution
Do not reject entire observations based on price. Prices being valid in
an observation does not increase trustworthiness of its interval.

Instead, we can reject a token if len(not_nil(obs[token])l) < f + 1.
Reasons:
1. If len(not_nil(obs[token])) >= f + 1, then [1, f+1] honest node has
included the token, therefore we know this is a legit token
2. Knowing this is a legit token, taking the median of prices aggregated
from 2f+1 obs is sufficient to guard against f faults.
3. Validation cannot require more than f + 1 obs, that’d allow <f faulty
nodes to halt price reporting for the token

---------

Co-authored-by: Rens Rooimans <github@rensrooimans.nl>
asoliman92 pushed a commit that referenced this pull request Jul 31, 2024
## Motivation
Removal of static token price changes a node’s view of destTokens. The
prices reported by nodes that accepted new job specs will contain fewer
tokens than the prices reported by nodes that haven’t, each release will
reject the observations from the other.

## Solution
Do not reject entire observations based on price. Prices being valid in
an observation does not increase trustworthiness of its interval.

Instead, we can reject a token if len(not_nil(obs[token])l) < f + 1.
Reasons:
1. If len(not_nil(obs[token])) >= f + 1, then [1, f+1] honest node has
included the token, therefore we know this is a legit token
2. Knowing this is a legit token, taking the median of prices aggregated
from 2f+1 obs is sufficient to guard against f faults.
3. Validation cannot require more than f + 1 obs, that’d allow <f faulty
nodes to halt price reporting for the token

---------

Co-authored-by: Rens Rooimans <github@rensrooimans.nl>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants