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

fix: switch proposer_duties table primary key to slot_number #43

Merged
merged 1 commit into from
Oct 7, 2024

Conversation

thedevbirb
Copy link
Contributor

As title says this PR changes the PRIMARY_KEY of the proposer_duties table from public_key to slot_number, and updates the logic of set_proposer_duties method of the PostgresDatabaseService accordingly.

The reason this change is needed is that, even if rare, it might occur that a validator public key is chosen twice for the next proposer duties which leads to the second record not being written on the database table because there is a conflict with the primary key. This edge case happens quite often in a devnet which has a very small validator set size compared to Mainnet.

The downstream consequence of it is that when processing a new head event inside process_head_event in the ChainEventUpdater (see below)

let new_duties = if event.slot % 8 == 0 {
match self.database.get_proposer_duties().await {
Ok(new_duties) => Some(new_duties),
Err(err) => {
error!(error = %err, "Failed to get proposer duties from db");
None
}
}
} else {
None
};
// Update local cache if new duties were fetched.
if let Some(new_duties) = &new_duties {
self.proposer_duties = new_duties.clone();
}
// Get the next proposer duty for the new slot.
let next_duty =
self.proposer_duties.iter().find(|duty| duty.slot == event.slot + 1).cloned();
let update =
ChainUpdate::SlotUpdate(SlotUpdate { slot: event.slot, new_duties, next_duty });
self.send_update_to_subscribers(update).await;
}

some proposer duties are missing which might lead to next_duty to being None. The latter is then read inside the submit_block/submit_header functions throwing the error BuilderApiError::ProposerDutyNotFound incorrectly.

Copy link
Collaborator

@0w3n-d 0w3n-d left a comment

Choose a reason for hiding this comment

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

We can't modify this V1 file because it will cause an error when it tries to apply it to an existing relay. Instead it will need to modify the proposer_duties table. Can you add a new file to migrations folder with the next number and modify the table there?

@thedevbirb thedevbirb force-pushed the fix/proposer-duties branch from 145d16a to bf46ca4 Compare October 7, 2024 14:41
@thedevbirb
Copy link
Contributor Author

We can't modify this V1 file because it will cause an error when it tries to apply it to an existing relay. Instead it will need to modify the proposer_duties table. Can you add a new file to migrations folder with the next number and modify the table there?

Sorry about that! I've seen that on the develop branch the latest migration would be V21 so I added that. I hope it's good now.

@0w3n-d 0w3n-d merged commit c447047 into gattaca-com:develop Oct 7, 2024
0 of 3 checks passed
@thedevbirb thedevbirb deleted the fix/proposer-duties branch October 8, 2024 07:55
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