Skip to content

fix(sequencer): fix app hash in horcrux sentries#1646

Merged
joroshiba merged 9 commits intomainfrom
joroshiba/horcrux-bs
Oct 15, 2024
Merged

fix(sequencer): fix app hash in horcrux sentries#1646
joroshiba merged 9 commits intomainfrom
joroshiba/horcrux-bs

Conversation

@joroshiba
Copy link
Member

Summary

Changes the comparison when deciding if we need to re-execute a proposal to use the validator address and the timestamp as identifiers, to enable horcrux sentry nodes to not have app hash clashes.

Background

When running horcrux all sentry nodes act as the same validator and all nodes create a new proposal, but the signer only signs one of them. This creates issues since all sentry nodes except the signed node will not reexecute the proposal since it was created by their validator address even though the proposals are different. Timestamps are used to add additional fingerprinting to this without adding high computation cost.

Changes

  • Utilize timestamp as part of identifying whether to clear and execute proposal in prepare proposal

Testing

A validator reported this issue when running testnet, and similar changes were tested and validated.

@joroshiba joroshiba requested review from SuperFluffy and noot October 8, 2024 16:50
@github-actions github-actions bot added the sequencer pertaining to the astria-sequencer crate label Oct 8, 2024
@joroshiba joroshiba marked this pull request as ready for review October 8, 2024 16:50
@joroshiba joroshiba requested a review from a team as a code owner October 8, 2024 16:50
@joroshiba joroshiba enabled auto-merge October 15, 2024 16:12
@joroshiba joroshiba added this pull request to the merge queue Oct 15, 2024
Merged via the queue into main with commit 10441af Oct 15, 2024
@joroshiba joroshiba deleted the joroshiba/horcrux-bs branch October 15, 2024 16:33
steezeburger added a commit that referenced this pull request Oct 16, 2024
* main:
  feat(sequencer)!: rework all fees (#1647)
  fix(sequencer): fix app hash in horcrux sentries (#1646)
  fix(composer)!: update to work with appside mempool (#1643)
  fix(charts): rollup application monitoring (#1601)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

sequencer pertaining to the astria-sequencer crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants