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

feat: docs for sendRawTranscationConditional #956

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

hamdiallam
Copy link

Description

This is split between different sections

  1. protocol docs for how this method is implemented in the execution engine
  2. operator docs for the op-txproxy service that should be applied in front of this method
  3. account abstraction docs for bundlers integrating this endpoint

Additional context

Inspiration for this method comes from this design: https://notes.ethereum.org/@yoav/SkaX2lS9j

This design is also implemented by Arbitrum and Polygon

Copy link

netlify bot commented Oct 5, 2024

Deploy Preview for docs-optimism ready!

Name Link
🔨 Latest commit 9f99f1d
🔍 Latest deploy log https://app.netlify.com/sites/docs-optimism/deploys/67041acd185d450008ce3b71
😎 Deploy Preview https://deploy-preview-956--docs-optimism.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@hamdiallam
Copy link
Author

Links failures look unrelated to this PR

@hamdiallam hamdiallam marked this pull request as ready for review October 7, 2024 19:05
@hamdiallam hamdiallam requested review from 0xmariniere and a team as code owners October 7, 2024 19:05
Copy link
Contributor

coderabbitai bot commented Oct 7, 2024

Walkthrough

The pull request introduces several updates across multiple documentation files related to account abstraction and transaction handling in a blockchain context. Key changes include the addition of a new section on "Bundlers" in the account abstraction documentation, the introduction of a new passthrough proxy service for transaction handling, and the documentation of the eth_sendRawTransactionConditional RPC method. Additionally, JSON metadata files have been updated to reflect new features and methods, while a text file has been modified for consistency in identifier casing.

Changes

File Path Change Summary
pages/builders/tools/build/account-abstraction.mdx New section added: ## Bundlers discussing eth_sendRawTransactionConditional RPC method and its configuration.
pages/stack/operators/features/_meta.json Key added: "op-txproxy": "op-txproxy"; newline added at the end of the file.
pages/stack/operators/features/op-txproxy.mdx New documentation for passthrough proxy service, detailing eth_sendRawTransactionConditional method and its constraints.
pages/stack/protocol/features/_meta.json Key added: "send-raw-transaction-conditional": "SendRawTransactionConditional"; existing key "alt-da-mode" updated for proper formatting.
pages/stack/protocol/features/send-raw-transaction-conditional.mdx New document added for eth_sendRawTransactionConditional RPC method, outlining its structure, usage, and requirements.
words.txt New entries added for "Flashbots" and "txproxy"; multiple existing entries updated for casing consistency.

Possibly related PRs

Suggested labels

documentation, flag:merge-pending-release

Suggested reviewers

  • sbvegan
  • krofax

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 9

🧹 Outside diff range and nitpick comments (11)
pages/stack/protocol/features/_meta.json (1)

3-4: LGTM! Consider standardizing the casing style.

The changes look good and align with the PR objectives. The addition of the "send-raw-transaction-conditional" entry is appropriate for documenting the new sendRawTranscationConditional method.

Consider standardizing the casing style for consistency. Currently, "Alt-DA Mode" uses title case, while "SendRawTransactionConditional" uses PascalCase. You might want to choose one style and apply it consistently across all entries.

 {
     "custom-gas-token": "Custom Gas Token",
-    "alt-da-mode": "Alt-DA Mode",
-    "send-raw-transaction-conditional": "SendRawTransactionConditional"
+    "alt-da-mode": "AltDAMode",
+    "send-raw-transaction-conditional": "SendRawTransactionConditional"
 }
words.txt (1)

Line range hint 1-358: Overall consistency improved, but casing rules could be clarified.

The changes to the casing of various terms throughout the file have improved overall consistency. However, it would be beneficial to clarify the rules for determining whether a term should be in all caps (e.g., ACCOUNTQUEUE, BLOOMFILTER) or just capitalized (e.g., Allocs, Predeploys).

Consider adding a comment at the beginning of the file explaining the casing conventions used. This will help maintain consistency in future updates.

pages/stack/protocol/features/send-raw-transaction-conditional.mdx (6)

11-16: Clarify terminology and maintain consistent capitalization.

The introduction is informative, but there are a couple of points that could be improved:

  1. The term "4337" might not be familiar to all readers. Consider providing a brief explanation or link to EIP-4337 for context.
  2. The capitalization of "UserOperations" is inconsistent with typical naming conventions. Consider using "userOperations" or "user operations" for better readability.

Consider updating the bullet point to:

* EIP-4337 bundlers utilizing a shared mempool of userOperations. Reverted transactions due to conflicting userOperations would make it too costly for bundlers to operate on L2, forcing the use of private mempools.

35-39: Improve grammar and clarify terminology.

There are two minor improvements that can be made to enhance clarity:

  1. In the first sentence of the Callout, "disincentive" should be "disincentivize".
  2. The term "MEV" (Maximal Extractable Value) might not be familiar to all readers. Consider providing a brief explanation or link for context.

Consider updating the Callout content to:

Since the sequencer is not compensated for the additional state checks, otherwise through the GAS of the transaction, a configured rate limit is applied to this cost.

To also disincentivize the use of this endpoint for MEV (Maximal Extractable Value) in comparison to `eth_sendRawTransaction`, the conditional is checked against the parent of the latest block.

41-45: Enhance readability of error codes.

To improve the readability and structure of the error code information, consider using a code block or a table format.

Here's a suggested format using a code block:

This conditional is checked once at the RPC layer before mempool submission. If rejected against chain state, the RPC will return an error with the following spec:

error code -32003: transaction rejected
Reason: specific failed check
error code -32005: conditional cost
Reason: conditional cost exceeded the maximum OR
cost was rate limited due to network conditions

🧰 Tools
🪛 LanguageTool

[style] ~41-~41: ‘prior to’ might be wordy. Consider a shorter alternative.
Context: ...tional is checked once at the RPC layer prior to mempool submission. If rejected against...

(EN_WORDINESS_PREMIUM_PRIOR_TO)


54-60: Maintain consistent capitalization for technical terms.

For better consistency and adherence to technical writing standards, consider using lowercase for the data types "boolean" and "integer".

Update the bullet points as follows:

* `--rollup.sequencertxconditionalenabled` (default: false) a boolean flag which enables the RPC.
* `--rollup.sequencertxconditionalcostratelimit` (default: 5000) an integer flag that sets the rate limit for cost observable per second.

61-63: Clarify terminology for broader audience understanding.

The Callout provides crucial security information. However, the term "DoS" (Denial of Service) might not be immediately clear to all readers.

Consider updating the Callout content to:

It is not advised to publicly expose this sequencer endpoint due to Denial of Service (DoS) concerns. This supplemental proxy, [op-txproxy](/stack/operators/features/op-txproxy), should be used to apply additional constraints on this endpoint prior to passing through to the sequencer.

41-41: Simplify wording for improved readability.

The phrase "prior to" can be replaced with a simpler alternative to enhance readability.

Consider updating the sentence to:

This conditional is checked once at the RPC layer before mempool submission.
🧰 Tools
🪛 LanguageTool

[style] ~41-~41: ‘prior to’ might be wordy. Consider a shorter alternative.
Context: ...tional is checked once at the RPC layer prior to mempool submission. If rejected against...

(EN_WORDINESS_PREMIUM_PRIOR_TO)

pages/stack/operators/features/op-txproxy.mdx (2)

44-48: Enhance the Rate Limits section with additional information.

The Rate Limits section provides crucial information about where rate limits are applied and how to configure them. However, it could benefit from additional details to provide a more comprehensive understanding.

Consider expanding this section with the following information:

  1. Explain the default rate limit value (5000) - is this per second, minute, or hour?
  2. Provide guidance on how to determine an appropriate rate limit for different scenarios.
  3. Describe what happens when the rate limit is exceeded (e.g., are requests rejected with a specific error code?).

For example:

#### Rate Limits

Even though the op-geth implementation of this endpoint includes rate limits, they are instead applied here to terminate these requests early.

The rate limit can be configured using:

`--sendRawTxConditional.ratelimit (default: 5000 requests per minute) ($OP_TXPROXY_SENDRAWTXCONDITIONAL_RATELIMIT)`

When determining an appropriate rate limit, consider factors such as your expected transaction volume and system capacity. If the rate limit is exceeded, requests will be rejected with a 429 (Too Many Requests) HTTP status code.

This additional information would provide operators with a more complete understanding of the rate limiting feature.


78-87: Improve step numbering and clarity in the build instructions.

The build instructions provide clear guidance for both building the binary and using the Docker image. However, there's a minor inconsistency in the numbering of sub-steps, and the clarity can be improved.

Please consider the following changes:

  1. Remove the numbering from the sub-steps to maintain consistency with the Steps component structure.
  2. Add a clear distinction between building the binary and pulling the Docker image.

Here's a suggested revision:

### Build the Binary or Pull the Docker Image

To build the binary:
```bash
make build

This will build and output the binary under /bin/op-txproxy.

Alternatively, you can use the Docker image:
The image for this binary is available as a docker artifact.


These changes improve the structure and clarity of the instructions, making it easier for users to follow whether they choose to build from source or use the Docker image.

</blockquote></details>
<details>
<summary>pages/builders/tools/build/account-abstraction.mdx (1)</summary><blockquote>

`22-30`: **Improve adherence to documentation guidelines**

The new "Bundlers" section provides valuable information about the `eth_sendRawTransactionConditional` RPC method. However, a few adjustments can enhance its alignment with our documentation guidelines:

1. Use proper nouns instead of "the stack" in the callout. Consider rephrasing to "As of today, this endpoint is not enabled by default in the OP Stack."

2. Apply proper title case to the header "Bundlers."

3. Use the imperative form in the second paragraph. Consider changing "If enabled by the chain operator, also see..." to "See the supplemental..."

4. Ensure consistent capitalization of "op-geth" and "op-txproxy."


Consider applying these changes to improve consistency and clarity:

```diff
-## Bundlers
+## Bundlers

-The OP Stack includes support for the `eth_sendRawTransactionConditional` RPC method to assist bundlers on shared 4337 mempools. See the [specification](/stack/protocol/features/send-raw-transaction-conditional) for how this method is implemented in op-geth.
+The OP Stack includes support for the `eth_sendRawTransactionConditional` RPC method to assist bundlers on shared 4337 mempools. See the [specification](/stack/protocol/features/send-raw-transaction-conditional) for how this method is implemented in Op-Geth.

-If enabled by the chain operator, also see the supplemental [op-txproxy](/stack/operators/features/op-txproxy) service, if applied, as this enforces request authentication for this method.
+See the supplemental [Op-TxProxy](/stack/operators/features/op-txproxy) service, if enabled by the chain operator, as this enforces request authentication for this method.

<Callout type="info">
-    As of today, this endpoint is not enabled by default in the stack. The operator must explicitly configure this.
+    As of today, this endpoint is not enabled by default in the OP Stack. The operator must explicitly configure this.
</Callout>
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 0be241d and 9f99f1d.

📒 Files selected for processing (6)
  • pages/builders/tools/build/account-abstraction.mdx (1 hunks)
  • pages/stack/operators/features/_meta.json (1 hunks)
  • pages/stack/operators/features/op-txproxy.mdx (1 hunks)
  • pages/stack/protocol/features/_meta.json (1 hunks)
  • pages/stack/protocol/features/send-raw-transaction-conditional.mdx (1 hunks)
  • words.txt (2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
pages/builders/tools/build/account-abstraction.mdx (1)

Pattern **/*.mdx: "ALWAYS review Markdown content THOROUGHLY with the following criteria:

  • Use proper nouns in place of personal pronouns like 'We' and 'Our' to maintain consistency in communal documentation.
  • Avoid gender-specific language and use the imperative form.
  • Monitor capitalization for emphasis. Use bold for prominence instead of all caps or italics.
  • Ensure proper nouns are capitalized in sentences.
  • Apply the Oxford comma.
  • Use proper title case for headers, buttons, tab names, page names, and links. Sentence case should be used for body content and short phrases, even in links.
  • Use correct spelling and grammar at all times (IMPORTANT).
    "
pages/stack/operators/features/op-txproxy.mdx (1)

Pattern **/*.mdx: "ALWAYS review Markdown content THOROUGHLY with the following criteria:

  • Use proper nouns in place of personal pronouns like 'We' and 'Our' to maintain consistency in communal documentation.
  • Avoid gender-specific language and use the imperative form.
  • Monitor capitalization for emphasis. Use bold for prominence instead of all caps or italics.
  • Ensure proper nouns are capitalized in sentences.
  • Apply the Oxford comma.
  • Use proper title case for headers, buttons, tab names, page names, and links. Sentence case should be used for body content and short phrases, even in links.
  • Use correct spelling and grammar at all times (IMPORTANT).
    "
pages/stack/protocol/features/send-raw-transaction-conditional.mdx (1)

Pattern **/*.mdx: "ALWAYS review Markdown content THOROUGHLY with the following criteria:

  • Use proper nouns in place of personal pronouns like 'We' and 'Our' to maintain consistency in communal documentation.
  • Avoid gender-specific language and use the imperative form.
  • Monitor capitalization for emphasis. Use bold for prominence instead of all caps or italics.
  • Ensure proper nouns are capitalized in sentences.
  • Apply the Oxford comma.
  • Use proper title case for headers, buttons, tab names, page names, and links. Sentence case should be used for body content and short phrases, even in links.
  • Use correct spelling and grammar at all times (IMPORTANT).
    "
🪛 LanguageTool
pages/stack/operators/features/op-txproxy.mdx

[uncategorized] ~11-~11: Possible missing article found.
Context: ...the ingress router should only re-route request for these methods. <Callout type="info...

(AI_HYDRA_LEO_MISSING_THE)


[style] ~21-~21: Style-wise, it’s not ideal to insert an adverb (‘horizontally’) in the middle of an infinitive construction (‘to scale’). Try moving the adverb to avoid split infinitives.
Context: ...y, additional stateless constraints are applied to horizontally scale validation rules and preemptively reject conditional tra...

(SPLIT_INFINITIVE)


[style] ~21-~21: ‘prior to’ might be wordy. Consider a shorter alternative.
Context: ...ptively reject conditional transactions prior to reaching the sequencer. A variety of m...

(EN_WORDINESS_PREMIUM_PRIOR_TO)


[style] ~22-~22: The phrase “A variety of” may be wordy. To make your writing clearer, consider replacing it.
Context: ...tions prior to reaching the sequencer. A variety of metrics are emitted for to inform adjus...

(A_VARIETY_OF)


[grammar] ~23-~23: Make sure the noun ‘variety’ is in agreement with the verb ‘are’. Beware that some collective nouns (like ‘police’ or ‘team’) can be treated as both singular and plural.
Context: ...ng the sequencer. A variety of metrics are emitted for to inform adjustments to be...

(DT_NN_OF_NNS_VB)


[style] ~34-~34: In American English, abbreviations like “etc.” require a period.
Context: ...cies (allowlist, localized rate limits, etc) #### Runtime Shutoff This service ca...

(ETC_PERIOD)


[uncategorized] ~38-~38: Possible missing article found.
Context: ...rolled with a flag/env switch to reject conditional transaction without needing to interrup...

(AI_HYDRA_LEO_MISSING_A)


[uncategorized] ~53-~53: The abbreviation “i.e.” (= that is) requires two periods.
Context: ...e max * Conditional values are valid (i.e min < max) * Transaction target are ...

(I_E)

pages/stack/protocol/features/send-raw-transaction-conditional.mdx

[style] ~41-~41: ‘prior to’ might be wordy. Consider a shorter alternative.
Context: ...tional is checked once at the RPC layer prior to mempool submission. If rejected against...

(EN_WORDINESS_PREMIUM_PRIOR_TO)

🔇 Additional comments (4)
pages/stack/protocol/features/_meta.json (1)

5-5: LGTM!

The closing brace is correctly placed, maintaining the JSON structure.

words.txt (2)

107-107: LGTM: Addition of "Flashbots" is appropriate.

The inclusion of "Flashbots" is relevant to the blockchain ecosystem and aligns with the PR's focus on transaction handling documentation.


358-358: LGTM: Addition of "txproxy" is relevant.

The inclusion of "txproxy" directly supports the documentation of the new passthrough proxy service for transaction handling, as mentioned in the PR objectives.

pages/stack/operators/features/op-txproxy.mdx (1)

73-77: LGTM: Clear introduction to the usage steps.

The introduction to the "How it works" section is well-structured and effectively sets up the following steps for using op-txproxy. The use of the Steps component from nextra is appropriate for presenting a sequence of actions.

Comment on lines 0 to 4
{
"proxyd": "proxyd"
} No newline at end of file
"proxyd": "proxyd",
"op-txproxy": "op-txproxy"
}
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Missing Documentation for op-txproxy

No documentation for "op-txproxy" was found in the repository. Please ensure that documentation for this new feature is created and properly integrated.

🔗 Analysis chain

LGTM! New feature added correctly.

The addition of the "op-txproxy" entry is consistent with the existing structure and naming convention. The JSON remains valid, and the newline at the end of the file is a good practice.

To ensure completeness, let's verify if documentation for the new "op-txproxy" feature exists:


Action Required: Verify Existence of op-txproxy Documentation

It appears that no Markdown documentation for "op-txproxy" was found. To ensure comprehensive verification, please run the following scripts to search for documentation in other formats and locations:


Action Required: Comprehensive Verification of op-txproxy Documentation

Previous searches did not locate any documentation for "op-txproxy". To ensure that documentation exists and is correctly placed, please run the following scripts to perform a comprehensive search across the entire repository:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for the existence of op-txproxy documentation

# Test: Search for op-txproxy documentation file
fd -e md op-txproxy

# Test: Search for op-txproxy mentions in markdown files
rg --type md "op-txproxy"

Length of output: 47


Script:

#!/bin/bash
# Description: Search for op-txproxy documentation in various formats and directories

# Search for documentation files with different extensions
fd -e md -e txt -e rst op-txproxy

# Search for mentions of op-txproxy in documentation directories
rg --type md --type txt --type rst "op-txproxy" docs/ documentation/

Length of output: 205


Script:

#!/bin/bash
# Description: Comprehensive search for op-txproxy documentation across the repository

# Search for any files containing "op-txproxy" in various documentation and code-related file types
rg "op-txproxy" --type-add 'misc:*.{md,txt,rst,js,yaml,json,py}' --type misc

Length of output: 149

Comment on lines +1 to +5
---
title: SendRawTransactionConditional Explainer
lang: en-US
description: Learn the basic process, benefits, and considerations for running a custom gas token chain.
---
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Update the description to accurately reflect the document content.

The current description mentions "running a custom gas token chain," which is not related to the content of this document. Please revise it to accurately describe the SendRawTransactionConditional feature.

Consider updating the description to something like:

description: Learn about the eth_sendRawTransactionConditional RPC method for conditional transaction inclusion in Optimism.

Comment on lines +50 to +60
#### Stateless Validation

* Conditional cost is below the max
* Conditional values are valid (i.e min \< max)
* Transaction target are only 4337 Entrypoint contracts

<Callout type="info">
The motivating factor for this endpoint is to enable permissionless 4337 mempools, hence the restricted usage of this methods to just [Entrypoint](https://github.com/eth-infinitism/account-abstraction/blob/develop/contracts/core/EntryPoint.sol) transactions.

Please open up an issue if you'd like this restriction to be optional via configuration to broaden usage of this endpoint.
</Callout>
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Improve formatting and clarity in the Stateless Validation section.

The Stateless Validation section provides important information about the validation rules. However, there's a minor formatting issue in the bullet points, and the clarity can be improved.

Please apply the following changes to improve formatting and clarity:

-*   Conditional cost is below the max
-*   Conditional values are valid (i.e min \< max)
-*   Transaction target are only 4337 Entrypoint contracts
+* Conditional cost is below the maximum allowed value
+* Conditional values are valid (i.e., min < max)
+* Transaction targets are only 4337 Entrypoint contracts

These changes improve the formatting consistency and clarity of the bullet points.

Consider adding brief explanations or examples for each validation rule to enhance understanding. For instance:

* Conditional cost is below the maximum allowed value (e.g., gas price * gas limit < maxConditionalCost)
* Conditional values are valid (i.e., min < max for all conditional parameters)
* Transaction targets are only 4337 Entrypoint contracts (to ensure compatibility with account abstraction)

This additional context would help readers better understand the purpose and implementation of each validation rule.

🧰 Tools
🪛 LanguageTool

[uncategorized] ~53-~53: The abbreviation “i.e.” (= that is) requires two periods.
Context: ...e max * Conditional values are valid (i.e min < max) * Transaction target are ...

(I_E)

Comment on lines +62 to +71
When the request passes validation, it is passed through to the configured backend URL

`--sendRawTxConditional.backend ($OP_TXPROXY_SENDRAWTXCONDITIONAL_BACKENDS)`


<Callout type="warning">
Per the [specification](/stack/protocol/features/send-raw-transaction-conditional), conditional transactions are not gossip'd between peers. Thus if you use replicas in an active/passive sequencer setup, this request must be broadcasted to all replicas.

[proxyd](./proxyd) as an egress router for this method supports this broadcasting functionality.
</Callout>
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Correct the grammatical error and enhance clarity in the backend configuration section.

The backend configuration and warning provide crucial information for operators. However, there's a minor grammatical issue that needs attention, and the clarity can be improved.

Please apply the following change in the warning message:

-Per the [specification](/stack/protocol/features/send-raw-transaction-conditional), conditional transactions are not gossip'd between peers. Thus if you use replicas in an active/passive sequencer setup, this request must be broadcasted to all replicas.
+Per the [specification](/stack/protocol/features/send-raw-transaction-conditional), conditional transactions are not gossiped between peers. Thus, if you use replicas in an active/passive sequencer setup, this request must be broadcast to all replicas.

This change corrects the grammatical errors ("gossip'd" to "gossiped" and "broadcasted" to "broadcast") and adds a comma after "Thus" for better readability.

Consider adding a brief explanation of why conditional transactions are not gossiped between peers and the implications of this for operators. This would provide valuable context for the warning. For example:

<Callout type="warning">
  Per the [specification](/stack/protocol/features/send-raw-transaction-conditional), conditional transactions are not gossiped between peers to prevent potential network congestion and maintain transaction privacy. As a result, if you use replicas in an active/passive sequencer setup, this request must be broadcast to all replicas to ensure consistency across your network.

  [proxyd](./proxyd) as an egress router for this method supports this broadcasting functionality.
</Callout>

This additional context helps operators understand the reasoning behind the warning and its importance.

Comment on lines +88 to +101
### Configure

The binary accepts configuration through cli flags which also settable via ENV variables. Either set the flags explicitly when starting the binary or set the environment variables of the host starting the proxy

See [methods](#methods) on the configuration options available for each method.

### Start

start the service with the following command

```bash
op-txproxy // ... with flags if env variables are not set
```
</Steps>
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Correct grammatical error and enhance clarity in the Configure and Start steps.

The Configure and Start steps provide essential information for setting up and running the service. However, there's a minor grammatical issue that needs attention, and the clarity can be improved.

Please apply the following change in the Configure step:

-The binary accepts configuration through cli flags which also settable via ENV variables. Either set the flags explicitly when starting the binary or set the environment variables of the host starting the proxy
+The binary accepts configuration through CLI flags, which are also settable via environment variables. Either set the flags explicitly when starting the binary or set the environment variables on the host starting the proxy.

This change corrects the grammatical errors and improves clarity.

Consider adding an example of how to set a configuration option both as a CLI flag and as an environment variable. This would provide a clearer illustration for users. For example:

### Configure

The binary accepts configuration through CLI flags, which are also settable via environment variables. Either set the flags explicitly when starting the binary or set the environment variables on the host starting the proxy.

Example:
- CLI flag: `--sendRawTxConditional.ratelimit 1000`
- Environment variable: `export OP_TXPROXY_SENDRAWTXCONDITIONAL_RATELIMIT=1000`

See the [Methods](#methods) section for the configuration options available for each method.

### Start

Start the service with the following command:

```bash
op-txproxy # Add any necessary flags if environment variables are not set

These changes provide more concrete examples and improve the overall clarity of the instructions.

<!-- This is an auto-generated comment by CodeRabbit -->

Comment on lines +17 to +24
## Methods

### **eth_sendRawTransactionConditional**

To safely expose this endpoint publicly, additional stateless constraints are applied to horizontally scale validation rules and preemptively reject conditional transactions prior to reaching the sequencer.

A variety of metrics are emitted for to inform adjustments to be made.

Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Correct the grammatical error and improve clarity in the Methods section.

The introduction to the eth_sendRawTransactionConditional method is informative. However, there's a grammatical issue that needs attention, and the clarity can be improved.

Please apply the following changes:

-A variety of metrics are emitted for to inform adjustments to be made.
+A variety of metrics are emitted to inform necessary adjustments.

This change corrects the grammatical error and improves the clarity of the sentence.

Consider adding a brief explanation of what "stateless constraints" means in this context to enhance understanding for readers who might be unfamiliar with the term.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
## Methods
### **eth_sendRawTransactionConditional**
To safely expose this endpoint publicly, additional stateless constraints are applied to horizontally scale validation rules and preemptively reject conditional transactions prior to reaching the sequencer.
A variety of metrics are emitted for to inform adjustments to be made.
## Methods
### **eth_sendRawTransactionConditional**
To safely expose this endpoint publicly, additional stateless constraints are applied to horizontally scale validation rules and preemptively reject conditional transactions prior to reaching the sequencer.
A variety of metrics are emitted to inform necessary adjustments.
🧰 Tools
🪛 LanguageTool

[style] ~21-~21: Style-wise, it’s not ideal to insert an adverb (‘horizontally’) in the middle of an infinitive construction (‘to scale’). Try moving the adverb to avoid split infinitives.
Context: ...y, additional stateless constraints are applied to horizontally scale validation rules and preemptively reject conditional tra...

(SPLIT_INFINITIVE)


[style] ~21-~21: ‘prior to’ might be wordy. Consider a shorter alternative.
Context: ...ptively reject conditional transactions prior to reaching the sequencer. A variety of m...

(EN_WORDINESS_PREMIUM_PRIOR_TO)


[style] ~22-~22: The phrase “A variety of” may be wordy. To make your writing clearer, consider replacing it.
Context: ...tions prior to reaching the sequencer. A variety of metrics are emitted for to inform adjus...

(A_VARIETY_OF)


[grammar] ~23-~23: Make sure the noun ‘variety’ is in agreement with the verb ‘are’. Beware that some collective nouns (like ‘police’ or ‘team’) can be treated as both singular and plural.
Context: ...ng the sequencer. A variety of metrics are emitted for to inform adjustments to be...

(DT_NN_OF_NNS_VB)

Comment on lines +36 to +43
#### Runtime Shutoff

This service can be rolled with a flag/env switch to reject conditional transaction without needing to interrupt the execution engine. Useful in diagnosing issues.

`--sendRawTxConditional.enabled (default: true) ($OP_TXPROXY_SENDRAWTXCONDITIONAL_ENABLED)`

When disabled, requests will fail with the `-32003` (transaction rejected) json rpc error code with a message stating that the method is disabled.

Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Correct the grammatical error and improve clarity in the Runtime Shutoff section.

The Runtime Shutoff section provides valuable information for operators. However, there's a minor grammatical issue that needs attention, and the clarity can be improved.

Please apply the following change:

-This service can be rolled with a flag/env switch to reject conditional transaction without needing to interrupt the execution engine. Useful in diagnosing issues.
+This service can be configured with a flag or environment variable to reject conditional transactions without needing to interrupt the execution engine. This feature is useful for diagnosing issues.

This change corrects the grammatical error (singular "transaction" to plural "transactions"), improves clarity by using more precise language, and connects the two sentences for better flow.

Consider adding a brief example of when an operator might want to use this feature to provide more context for its usefulness.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
#### Runtime Shutoff
This service can be rolled with a flag/env switch to reject conditional transaction without needing to interrupt the execution engine. Useful in diagnosing issues.
`--sendRawTxConditional.enabled (default: true) ($OP_TXPROXY_SENDRAWTXCONDITIONAL_ENABLED)`
When disabled, requests will fail with the `-32003` (transaction rejected) json rpc error code with a message stating that the method is disabled.
#### Runtime Shutoff
This service can be configured with a flag or environment variable to reject conditional transactions without needing to interrupt the execution engine. This feature is useful for diagnosing issues.
`--sendRawTxConditional.enabled (default: true) ($OP_TXPROXY_SENDRAWTXCONDITIONAL_ENABLED)`
When disabled, requests will fail with the `-32003` (transaction rejected) json rpc error code with a message stating that the method is disabled.
🧰 Tools
🪛 LanguageTool

[uncategorized] ~38-~38: Possible missing article found.
Context: ...rolled with a flag/env switch to reject conditional transaction without needing to interrup...

(AI_HYDRA_LEO_MISSING_A)

Comment on lines +1 to +16
---
title: op-txproxy
lang: en-US
description: A passthrough proxy service that can apply additional constraints on transactions prior to reaching the sequencer.
---

import { Callout, Steps } from 'nextra/components'

# op-txproxy

A [passthrough proxy](https://github.com/ethereum-optimism/infra/tree/main/op-txproxy) for the execution engine endpoint. This proxy does not forward all rpc traffic and only exposes a specific set of methods. Operationally, the ingress router should only re-route request for these methods.

<Callout type="info">
[proxyd](./proxyd) as an ingress router supports the mapping of specific methods to unique backends.
</Callout>

Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Correct the grammatical error in the introduction.

The introduction provides a clear overview of the op-txproxy service. However, there's a minor grammatical issue that needs correction.

Please apply the following change:

-A [passthrough proxy](https://github.com/ethereum-optimism/infra/tree/main/op-txproxy) for the execution engine endpoint. This proxy does not forward all rpc traffic and only exposes a specific set of methods. Operationally, the ingress router should only re-route request for these methods.
+A [passthrough proxy](https://github.com/ethereum-optimism/infra/tree/main/op-txproxy) for the execution engine endpoint. This proxy does not forward all rpc traffic and only exposes a specific set of methods. Operationally, the ingress router should only re-route requests for these methods.

The rest of the content in this segment looks good.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
---
title: op-txproxy
lang: en-US
description: A passthrough proxy service that can apply additional constraints on transactions prior to reaching the sequencer.
---
import { Callout, Steps } from 'nextra/components'
# op-txproxy
A [passthrough proxy](https://github.com/ethereum-optimism/infra/tree/main/op-txproxy) for the execution engine endpoint. This proxy does not forward all rpc traffic and only exposes a specific set of methods. Operationally, the ingress router should only re-route request for these methods.
<Callout type="info">
[proxyd](./proxyd) as an ingress router supports the mapping of specific methods to unique backends.
</Callout>
---
title: op-txproxy
lang: en-US
description: A passthrough proxy service that can apply additional constraints on transactions prior to reaching the sequencer.
---
import { Callout, Steps } from 'nextra/components'
# op-txproxy
A [passthrough proxy](https://github.com/ethereum-optimism/infra/tree/main/op-txproxy) for the execution engine endpoint. This proxy does not forward all rpc traffic and only exposes a specific set of methods. Operationally, the ingress router should only re-route requests for these methods.
<Callout type="info">
[proxyd](./proxyd) as an ingress router supports the mapping of specific methods to unique backends.
</Callout>
🧰 Tools
🪛 LanguageTool

[uncategorized] ~11-~11: Possible missing article found.
Context: ...the ingress router should only re-route request for these methods. <Callout type="info...

(AI_HYDRA_LEO_MISSING_THE)

Comment on lines +25 to +35
#### Authentication

The caller authenticates themselves with any valid ECDSA-secp256k1 key, like an Ethereum key. The computed signature is over the [EIP-191](https://eips.ethereum.org/EIPS/eip-191) hash of the request body. This calling address does **not need to hold an ethereum balance**. It simply is used for identification.

With the signature and signing address, the request is authenticated under the `X-Optimism-Signature` header with the value `<public key address>: <signature>`.

* Requests with missing a missing authentication header fail with the `-32003` (transaction rejected) json rpc error code.
* Requests with a mismatch in recovered signer and supplied public key will have the http request failed with status code `400 - Bad Request`.

As of today, no authorization policies are implemented on this endpoint. This has preemptively in place allow for new policies (allowlist, localized rate limits, etc)

Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Correct the typo and consider future authorization policies.

The Authentication section provides a clear explanation of the process. However, there's a minor typo that needs correction, and it might be beneficial to consider future authorization policies.

Please correct the following typo:

-*   Requests with missing a missing authentication header fail with the `-32003` (transaction rejected) json rpc error code.
+*   Requests with a missing authentication header fail with the `-32003` (transaction rejected) json rpc error code.

Regarding the statement about authorization policies:

As of today, no authorization policies are implemented on this endpoint. This has preemptively in place allow for new policies (allowlist, localized rate limits, etc)

Consider rephrasing this to make it clearer and more future-proof:

As of today, no authorization policies are implemented on this endpoint. However, the authentication mechanism is in place to allow for future implementation of policies such as allowlists, localized rate limits, etc.

This change clarifies the current state and the potential for future enhancements.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
#### Authentication
The caller authenticates themselves with any valid ECDSA-secp256k1 key, like an Ethereum key. The computed signature is over the [EIP-191](https://eips.ethereum.org/EIPS/eip-191) hash of the request body. This calling address does **not need to hold an ethereum balance**. It simply is used for identification.
With the signature and signing address, the request is authenticated under the `X-Optimism-Signature` header with the value `<public key address>: <signature>`.
* Requests with missing a missing authentication header fail with the `-32003` (transaction rejected) json rpc error code.
* Requests with a mismatch in recovered signer and supplied public key will have the http request failed with status code `400 - Bad Request`.
As of today, no authorization policies are implemented on this endpoint. This has preemptively in place allow for new policies (allowlist, localized rate limits, etc)
#### Authentication
The caller authenticates themselves with any valid ECDSA-secp256k1 key, like an Ethereum key. The computed signature is over the [EIP-191](https://eips.ethereum.org/EIPS/eip-191) hash of the request body. This calling address does **not need to hold an ethereum balance**. It simply is used for identification.
With the signature and signing address, the request is authenticated under the `X-Optimism-Signature` header with the value `<public key address>: <signature>`.
* Requests with a missing authentication header fail with the `-32003` (transaction rejected) json rpc error code.
* Requests with a mismatch in recovered signer and supplied public key will have the http request failed with status code `400 - Bad Request`.
As of today, no authorization policies are implemented on this endpoint. However, the authentication mechanism is in place to allow for future implementation of policies such as allowlists, localized rate limits, etc.
🧰 Tools
🪛 LanguageTool

[style] ~34-~34: In American English, abbreviations like “etc.” require a period.
Context: ...cies (allowlist, localized rate limits, etc) #### Runtime Shutoff This service ca...

(ETC_PERIOD)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant