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.

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

Previewnet temporary fix for missing timestamps #245

Merged
merged 1 commit into from
May 14, 2024
Merged

Previewnet temporary fix for missing timestamps #245

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
12 changes: 12 additions & 0 deletions api/api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -613,6 +613,18 @@ func (b *BlockChainAPI) prepareBlockResponse(
Timestamp: hexutil.Uint64(block.Timestamp),
}

// todo remove after previewnet, temp fix to mock some of the timestamps
if block.Timestamp == 0 {
first := uint64(1715189257)
blockTime := uint64(200)
firstRecordedTimestampBlock := uint64(5493)

diff := firstRecordedTimestampBlock - block.Height
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe just check diff doesn't get negative

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did in a local test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can add an if statement tho, but then what value to assign :D

timestamp := first - blockTime*diff
Copy link
Contributor

Choose a reason for hiding this comment

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

should this be + ? I also think you might multiply it by second.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It should be - because what I did is checked what is the first timestamp we have indexed, which is: 1715189257 then I subtract 200 (average block timestamp change I calculated) as many times as there is a diff from the height of the block that has this value and the block we are generating. Basically height 0 would have biggest change and height 5492 which is one less than first recorded timestamp would have lowest change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could have better documented, but this will all go away


blockResponse.Timestamp = hexutil.Uint64(timestamp)
}
Comment on lines +616 to +626
Copy link
Contributor

Choose a reason for hiding this comment

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

Ensure the temporary fix is clearly marked and documented.

The temporary fix for missing timestamps is clearly marked with a TODO comment. However, it would be beneficial to add more context about why this fix is necessary and any potential impacts. This will help future maintainers understand the reason behind this code and when it can be safely removed.

Consider adding a more detailed comment like this:

// TODO: Remove after previewnet, temp fix to mock some of the timestamps
// This temporary fix addresses the issue of missing timestamps in previewnet.
// It calculates a mock timestamp based on a predefined starting point and block time.
// This fix should be removed once the underlying issue is resolved.


transactions, err := b.fetchBlockTransactions(ctx, block)
if err != nil {
return nil, err
Expand Down
Loading