Skip to content

Shd/pointer slot check #52

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

Merged
merged 2 commits into from
Jun 30, 2025
Merged

Shd/pointer slot check #52

merged 2 commits into from
Jun 30, 2025

Conversation

shd
Copy link
Collaborator

@shd shd commented Jun 26, 2025

No description provided.

@@ -40,7 +42,27 @@ impl PointerCache {
self.pointer_map.insert(ptr, Some(addr));
}

pub fn ensure_up_to_date_ptr(&self, ptr: &ShelleyAddressPointer) -> Result<()> {
pub fn update_block(&mut self, blk: &BlockInfo) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Now we know what the start slot is, do we still need to capture it here? I guess it might be a good idea to check that what we thought it was actually is the case, though!

Copy link
Collaborator Author

@shd shd Jun 29, 2025

Choose a reason for hiding this comment

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

There are two parts of the module:

  1. Replaying already collected pointer cache.
  2. Creating pointer cache (e.g., for test network/sancho network). We do not know eras boundaries in that situation, and need to collect them.
    This code is used only in second situation (in handle_deltas call, which is used only in stateful init).


pub fn ensure_up_to_date_ptr(&self, blk: &BlockInfo, ptr: &ShelleyAddressPointer) -> Result<()> {
if ptr.slot > blk.slot {
// We believe that pointers cannot point forward
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggest a warn! here so we can at least know this has happened

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The idea is that we've already collected the necessary info about pointers, and here we just replay the collected info. A pointer cannot point to future -- so if it happens then our database is up-to-date in respect to this.

There are some forward pointers in blockchain: and we need to skip errors about them (that's the reason for these changes).


if let Some(conway_start_slot) = self.conway_start_slot {
if ptr.slot >= conway_start_slot {
// Conway epoch slots cannot be referenced
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ditto

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Do we really need to have warnings about incorrect pointers in blockchain? They will be shown again and again each invocation. Another idea: we can collect the errors and record errors that should be suppressed.

return Ok(());
}
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this test now redundant given we're testing against pointing forward above?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This test remains reasonable: if the database for pointers is not up-to-date, then we should write about that.

@sandtreader
Copy link
Collaborator

All understood, missed the different phases.

@sandtreader sandtreader reopened this Jun 30, 2025
@sandtreader
Copy link
Collaborator

Oops.

@sandtreader sandtreader merged commit 915de2d into main Jun 30, 2025
4 checks passed
@sandtreader sandtreader deleted the shd/pointer-slot-check branch June 30, 2025 12:08
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.

2 participants