BM-1620: Switch pvw commands to use ETH_RPC_URL to disambiguate deployments #1152
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
austinabell
requested review from
a team,
Wollac,
capossele,
willemolding and
zeroecco
as code owners
github-actions
bot
changed the title
willpote
approved these changes
Sep 23, 2025
derrix060
reviewed
Sep 29, 2025
| prover_config: ProverConfig, | ||
|
|
||
| /// The RPC URL to use to submit the reward claim transaction. | ||
| #[clap(long, env = "ETH_RPC_URL")] |
There was a problem hiding this comment.
nit: maybe rename it to REWARD_RPC_URL so in case it's decided to use a different chain for the reward (for example base due to tx fees in eth), it's a no brainer :)
There was a problem hiding this comment.
yeah agree that might be a better name. @willpote you good with this, also because similarity with us using REWARD_ADDRESS env vars now?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change is to avoid the footgun of mixing up RPC_URL from other commands, as the same variable is used to point to the market deployment and also for the accounting contract. Name isn't perfect, because very possible the accounting contract is not on Ethereum, and the market they are interacting on is also Ethereum, which could cause other confusions.
Naming suggestions welcome, couldn't think of one that would prevent the footguns we have been seeing more than this.