Skip to content

Conversation

DZakh
Copy link
Member

@DZakh DZakh commented Oct 10, 2025

No description provided.

Copy link
Contributor

coderabbitai bot commented Oct 10, 2025

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch dz/rollback-v2

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Comment on lines +390 to +393
| Some(_) => // Since the DC is registered by an earlier item in the query
// FIXME: This unsafely relies on the asc order of the items
// which is 99% true, but there were cases when the source ordering was wrong
false
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe add an assertion if possible or handle it so that there is no UB

// Currently history update table uses S.object with transformation for schema,
// which is being lossed during conversion to dbSchema.
// So use simple insert values for now.
let isHistoryUpdate = table.tableName->Js.String2.startsWith("envio_history_")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you rather make this an api on the table?

let getSafeCheckpointId = (safeCheckpointTracking: t, ~sourceBlockNumber: int) => {
let safeBlockNumber = sourceBlockNumber - safeCheckpointTracking.maxReorgDepth

if safeCheckpointTracking.checkpointBlockNumbers->Belt.Array.getUnsafe(0) > safeBlockNumber {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't use unsafe

Comment on lines +1020 to +1027
switch await Db.sql->InternalTable.Checkpoints.getRollbackTargetCheckpoint(
~pgSchema=Env.Db.publicSchema,
~reorgChainId,
~lastKnownValidBlockNumber,
) {
| [checkpoint] => checkpoint["id"]
| _ => 0
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

would put this switch inside the function

Copy link
Collaborator

@JonoPrest JonoPrest left a comment

Choose a reason for hiding this comment

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

⭐ 🚀

@DZakh DZakh mentioned this pull request Oct 21, 2025
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