Skip to content

chore: check against chain tip#2210

Open
SantiagoPittella wants to merge 1 commit into
nextfrom
santiagopittella-1866-validate-block-to-chain-tip
Open

chore: check against chain tip#2210
SantiagoPittella wants to merge 1 commit into
nextfrom
santiagopittella-1866-validate-block-to-chain-tip

Conversation

@SantiagoPittella
Copy link
Copy Markdown
Collaborator

closes #1866

Adds a check for the range to not go beyond the chain tip

Copy link
Copy Markdown
Collaborator

@Mirko-von-Leipzig Mirko-von-Leipzig left a comment

Choose a reason for hiding this comment

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

Thanks, just some nit suggestions.

I'm still against this overall, especially in a distributed setting it can be a problem if the client alternates between different nodes which may be at different chain heights. But lets see.

Just noting my disagreement again :D

///
/// Returns the chain tip so callers can reuse it (e.g. in the response's pagination info)
/// without issuing a second query.
async fn chain_tip_for_range(
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Maybe something like this?

Suggested change
async fn chain_tip_for_range(
async fn range_bounds_check(

Comment thread CHANGELOG.md

### RPC API

- Validated that `block_to` does not exceed the chain tip on all paginated sync endpoints ([#1866](https://github.com/0xMiden/node/issues/1866)).
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
- Validated that `block_to` does not exceed the chain tip on all paginated sync endpoints ([#1866](https://github.com/0xMiden/node/issues/1866)).
- RPC requests whose range exceed the chain tip are now rejected ([#1866](https://github.com/0xMiden/node/issues/1866)).

Copy link
Copy Markdown
Contributor

@kkovaacs kkovaacs left a comment

Choose a reason for hiding this comment

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

Looks good to me % Mirko's suggestions.

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.

Validate block_to is not greater than chain tip

3 participants