Skip to content

Mirror client fixes #1086

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.

Sign up for GitHub

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

Open
AlCutter wants to merge 1 commit into
base: main
Choose a base branch
from
+188 −94
Open

Mirror client fixes #1086

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions client/mirror/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,10 @@ func (c *Client) streamEntries(ctx context.Context, uploadStart, uploadEnd uint6
startIdx := curr % 256
endIdx := startIdx + numEntries

if uint64(len(bundle.Entries)) < endIdx {
_ = pw.CloseWithError(fmt.Errorf("bundle %d has only %d entries, expected at least %d", bundleIndex, len(bundle.Entries), endIdx))
return
}
for i := startIdx; i < endIdx; i++ {
entry := bundle.Entries[i]
if err := binary.Write(gw, binary.BigEndian, uint16(len(entry))); err != nil {
Expand Down Expand Up @@ -443,10 +447,14 @@ func (c *Client) buildCheckpointRequestBody(oldSize uint64, proof [][]byte, chec
// up to the specified targetSize. It returns the mirror's cosignatures on success.
func (c *Client) Sync(ctx context.Context, targetCheckpointRaw []byte, targetSize uint64) ([]byte, error) {
var conflict ErrConflict
nextEntry := c.oldSize
var nextEntry uint64

// Push the checkpoint with the old size (0 if not provided).
for c.oldSize < targetSize {
// Keep trying for as long we we get conflict errors or the context is not cancelled.

@roger2hk roger2hk Jul 31, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
// Keep trying for as long we we get conflict errors or the context is not cancelled.
// Keep trying for as long as we get conflict errors or the context is not cancelled.

// Ensure we send the checkpoint at least once, this serves two purposes:
// 1. We refresh the witness' timestamped view of the log, committing to the fact that the log hasn't grown.
// 2. If this is the first time we're pushing the checkpoint (i.e. c.oldSize == 0), we're ensuring that we'll send a zero-sized checkpoint.
for {

@roger2hk roger2hk Jul 31, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There is an infinite loop in the following scenario:

  • If the mirror's pending size is greater than targetSize (e.g., conflict.PendingSize=10 while targetSize=5), c.oldSize becomes 10.
  • In the next loop iteration, pushCheckpoint is called with c.oldSize=10 and targetSize=5.
  • The mirror rejects requests where uploadStart > uploadEnd and returns another ErrConflict error.
  • This Sync function continuously retries the failing request in an infinite loop.

err := c.pushCheckpoint(ctx, c.oldSize, targetSize, targetCheckpointRaw)
if err != nil {
if !errors.As(err, &conflict) {
Expand All @@ -458,6 +466,7 @@ func (c *Client) Sync(ctx context.Context, targetCheckpointRaw []byte, targetSiz

nextEntry = c.oldSize
c.oldSize = targetSize
break
}

// Push entries up to target size in packages of 256, handling concurrent conflicts and retries.
Expand Down
Loading
Loading