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

Add a boolean field to tx receipt object indicating if the tx was timeboosted #369

Merged
merged 11 commits into from
Jan 29, 2025

Conversation

ganeshvanahalli
Copy link
Contributor

@ganeshvanahalli ganeshvanahalli commented Oct 29, 2024

This PR introduces a new field to the receipts object/s returned by eth_getTransactionReceipt and eth_getBlockReceipts, indicating whether a transaction was timeboosted or not. This field is a boolean named as timeboosted.

The PR is based of of wasm-store-compile-target branch (revived) instead of master because the current main Timeboost PR's geth pin is pointing to this branch and we do this to be in sync with the main PR.

Corresponding nitro PR- OffchainLabs/nitro#2760
Resolves NIT-2836

internal/ethapi/api.go Outdated Show resolved Hide resolved
@ganeshvanahalli ganeshvanahalli changed the base branch from wasm-store-compile-target to add-methodsto-sizeconstrainedcache November 14, 2024 13:35
Tristan-Wilson
Tristan-Wilson previously approved these changes Nov 20, 2024
Base automatically changed from add-methodsto-sizeconstrainedcache to master December 30, 2024 19:19
@Tristan-Wilson Tristan-Wilson dismissed their stale review December 30, 2024 19:19

The base branch was changed.

eljobe
eljobe previously requested changes Jan 21, 2025
if txIndex >= maxTxCount {
return false, nil
}
return b[1+(txIndex/8)]&(1<<(txIndex%8)) != 0, nil
Copy link
Member

Choose a reason for hiding this comment

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

This line needs a comment.
8 seems like a magical number and it isn't clear how the bitwise & is checking if the transaction was timeboosted or not.

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 agree. Currently the explanation for this exists on the sequencer side where blockMetadata originates https://github.com/OffchainLabs/nitro/blob/071aa54de1a8c2f510b5d11835bad9124eb38f37/execution/gethexec/executionengine.go#L615-L619 but having it here as well is a good idea

Copy link
Collaborator

@tsahee tsahee left a comment

Choose a reason for hiding this comment

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

LGTM

@tsahee tsahee dismissed eljobe’s stale review January 29, 2025 01:00

adding a separate issue

@tsahee tsahee merged commit 8070936 into master Jan 29, 2025
8 checks passed
@tsahee tsahee deleted the include-timeboosted-receiptsfield branch January 29, 2025 01:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants