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 EIP: EVM Transaction Bundles #8684

Merged
merged 13 commits into from
Jul 9, 2024
Merged

Add EIP: EVM Transaction Bundles #8684

merged 13 commits into from
Jul 9, 2024

Conversation

Lilyjjo
Copy link
Contributor

@Lilyjjo Lilyjjo commented Jun 24, 2024

@Lilyjjo Lilyjjo requested a review from eth-bot as a code owner June 24, 2024 07:07
@github-actions github-actions bot added c-new Creates a brand new proposal s-draft This EIP is a Draft t-core labels Jun 24, 2024
@eth-bot
Copy link
Collaborator

eth-bot commented Jun 24, 2024

✅ All reviewers have approved.

@eth-bot eth-bot added e-consensus Waiting on editor consensus e-review Waiting on editor to review labels Jun 24, 2024
@eth-bot eth-bot changed the title Add EIP: Transaction Bundles Add EIP: EVM Transaction Bundles Jun 24, 2024
Copy link

The commit 515eeb1 (as a parent of d7360d0) contains errors.
Please inspect the Run Summary for details.

@github-actions github-actions bot added the w-ci Waiting on CI to pass label Jun 24, 2024
@github-actions github-actions bot removed the w-ci Waiting on CI to pass label Jun 25, 2024
EIPS/eip-7727.md Outdated Show resolved Hide resolved
EIPS/eip-7727.md Outdated Show resolved Hide resolved
EIPS/eip-7727.md Outdated Show resolved Hide resolved
EIPS/eip-7727.md Outdated Show resolved Hide resolved
EIPS/eip-7727.md Outdated Show resolved Hide resolved
EIPS/eip-7727.md Outdated Show resolved Hide resolved
EIPS/eip-7727.md Outdated
Comment on lines 133 to 143
The `BUNDLE_TX_TYPE` has a new gas cost formula. For the `BUNDLE_TX_TYPE` to be valid, it must have enough gas to cover:

1. The `BUNDLE_TX_TYPE` transaction base cost of `BUNDLE_BASE_GAS_COST`.
2. All bytes of the `transactionList` elements as if their bytes were part of a typical transaction’s `CALL_DATA`.

The final gas cost of the `BUNDLE_TX_TYPE` depends on if the `transactionList`'s elements are valid transactions or not. The following logic is used on the `transactionList` elements:

1. If an element is valid and can begin execution, the `BUNDLE_TX_TYPE` signer is not charged for that element’s bytes.
2. If the element is not valid and cannot being execution, the `BUNDLE_TX_TYPE` is charged as if that element’s bytes were just `CALL_DATA`.

At the start of processing, the `BUNDLE_TX_TYPE`'s signer must be charged as if all inner transactions were invalid and refunded the gas for the valid transactions as each valid transaction finishes execution.
Copy link
Contributor

Choose a reason for hiding this comment

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

Only include the actual gas formula in the specification section. The reasoning behind it should live in the rationale section. Helps to keep the specification as small and readable as possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mean I should have a code block instead of a verbal description? The words here are describing different parts of the gas costing logic and aren't justifications for it.

Copy link
Contributor

Choose a reason for hiding this comment

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

A code block would definitely be fine, but it isn't necessary. I think I just misinterpreted the way you wrote this, honestly.

Instead of writing it as "it must have enough gas to cover...", I'd recommend wording it like:

The gas cost of BUNDLE_TX_TYPE is calculated as follows BUNDLE_BASE_GAS_COST plus FOO times BAR, where FOO is zero if [...] and is seven otherwise.

Introducing the formula with "must have enough gas to cover" just doesn't sound precise enough.

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 updated it! Let me know if you think it still needs work, happy to refactor again.

EIPS/eip-7727.md Outdated Show resolved Hide resolved
EIPS/eip-7727.md Outdated Show resolved Hide resolved
@Lilyjjo
Copy link
Contributor Author

Lilyjjo commented Jun 27, 2024

Addressed all comments and left two questions.

@Lilyjjo Lilyjjo requested a review from SamWilsn June 27, 2024 07:15
gatleas17

This comment was marked as off-topic.

@ethereum ethereum deleted a comment from Parasos Jul 3, 2024
@eth-bot eth-bot enabled auto-merge (squash) July 9, 2024 14:39
Copy link
Collaborator

@eth-bot eth-bot left a comment

Choose a reason for hiding this comment

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

All Reviewers Have Approved; Performing Automatic Merge...

@eth-bot eth-bot merged commit 1868f75 into ethereum:master Jul 9, 2024
10 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c-new Creates a brand new proposal e-consensus Waiting on editor consensus e-review Waiting on editor to review s-draft This EIP is a Draft t-core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants