-
Notifications
You must be signed in to change notification settings - Fork 1k
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
add partial account filters for near #3669
Conversation
ae831df
to
701370d
Compare
701370d
to
dce8459
Compare
chain/near/src/adapter.rs
Outdated
// prefix [] and suffiex [c, d] would produce [None, c], [None, d] | ||
// prefix [a,b] and suffiex [] would produce [a, None], [b, None] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo here on suffix
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
// PrefixSuffixPair applies a logical AND to prefix and suffix when both fields are non-empty. | ||
// * {prefix="hello",suffix="world"} will match "hello.world" but not "hello.friend" | ||
// * {prefix="hello",suffix=""} will match both "hello.world" and "hello.friend" | ||
// * {prefix="",suffix="world"} will match both "hello.world" and "good.day.world" | ||
// * {prefix="",suffix=""} is invalid |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's nice to have docs here, thanks 👍
pub(crate) account: Option<String>, | ||
#[serde(rename = "startBlock", default)] | ||
pub(crate) start_block: BlockNumber, | ||
pub(crate) accounts: Option<PartialAccounts>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see that account
and accounts
are optional. What happens if one tries to deploy a NEAR subgraph that has both values as None
? Do you know if we have validation for this or if is it valid?
if no_source_address && has_receipt_handlers { | ||
|
||
// Validate not both address and partial addresses are empty. | ||
if (no_source_address && no_partial_addresses) && has_receipt_handlers { | ||
errors.push(SubgraphManifestValidationError::SourceAddressRequired.into()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nevermind about my other None
comment, the validation is here 🙂
chain/near/src/adapter.rs
Outdated
.map(|s| s.account.as_ref().cloned().unwrap()) | ||
.collect(); | ||
|
||
// Parse all the partial accounts, produces all possible combiantions of the values |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo in combinations
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
a03c891
to
7a3aa52
Compare
Fixes #3628