feat: add new contracts-periphery package#2536
Merged
mergify[bot] merged 2 commits intodevelopfrom May 16, 2022
Merged
Conversation
🦋 Changeset detectedLatest commit: 20b8eb3 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
|
This PR changes implementation code, but doesn't include a changeset. Did you forget to add one? |
Contributor
|
Going to need to add this to CI for publishing |
smartcontracts
force-pushed
the
sc/ct-periphery
branch
from
0dd1b94 to
79b2ce8
Compare
smartcontracts
force-pushed
the
sc/ct-periphery
branch
2 times, most recently
from
a5c7422 to
3d88dff
Compare
smartcontracts
force-pushed
the
sc/ct-periphery
branch
3 times, most recently
from
69871ce to
8618359
Compare
mslipper
approved these changes
May 15, 2022
tynes
reviewed
May 16, 2022
tynes
reviewed
May 16, 2022
packages/contracts-periphery/test/contracts/universal/RetroReceiver.spec.ts
Show resolved
Hide resolved
tynes
reviewed
May 16, 2022
tynes
reviewed
May 16, 2022
packages/contracts-periphery/test/contracts/universal/RetroReceiver.spec.ts
Show resolved
Hide resolved
tynes
reviewed
May 16, 2022
packages/contracts-periphery/test/contracts/universal/RetroReceiver.spec.ts
Outdated
Show resolved
Hide resolved
tynes
reviewed
May 16, 2022
Contributor
tynes
left a comment
tynes
left a comment
There was a problem hiding this comment.
Generally looks good to me, a few nits
Contributor
|
Hey @smartcontracts! This PR has merge conflicts. Please fix them before continuing review. |
smartcontracts
force-pushed
the
sc/ct-periphery
branch
from
8618359 to
6231622
Compare
Introduces a new contracts-periphery package and initializes the package with the RetroReceiver contract (meant to receive RetroPGF funds on all available chains). Reason for introducing a new package is that the original contracts package should be reserved for in-protocol contracts while contracts-periphery can be used for any contracts that we want to write and test, regardless of if they're part of the protocol.
Adds contracts-periphery package to CI in the various places it's required so we can successfully package and release it.
smartcontracts
force-pushed
the
sc/ct-periphery
branch
from
6231622 to
20b8eb3
Compare
tynes
approved these changes
May 16, 2022
Contributor
|
This PR has been added to the merge queue, and will be merged soon. |
Contributor
|
This PR is next in line to be merged, and will be merged as soon as checks pass. |
theochap
pushed a commit
that referenced
this pull request
Dec 10, 2025
…node CLI (#2536) ## Overview This PR adds comprehensive unit tests to verify that the `--l2-chain-id` CLI flag in `kona-node` correctly supports both string chain names and numeric chain IDs using `alloy_chains::Chain`. ## Background The `--l2-chain-id` flag was already implemented using `alloy_chains::Chain` instead of `u64`, but lacked comprehensive tests to verify that both string and numeric chain ID formats work correctly. This PR addresses that gap by adding thorough test coverage. ## Changes Made ### Unit Tests Added (`bin/node/src/flags/globals.rs`) - **`test_l2_chain_id_parse_numeric()`** - Verifies numeric chain ID parsing: - Optimism mainnet (`"10"` → chain ID 10) - Ethereum mainnet (`"1"` → chain ID 1) - Base mainnet (`"8453"` → chain ID 8453) - Custom/unknown chain IDs (`"999999"` → chain ID 999999) - **`test_l2_chain_id_parse_string()`** - Verifies string chain name parsing: - `"optimism"` → chain ID 10 - `"mainnet"` → chain ID 1 - `"base"` → chain ID 8453 - **`test_l2_chain_id_parse_invalid()`** - Verifies invalid inputs are rejected appropriately - **`test_l2_chain_id_short_flag()`** - Verifies short flag (`-c`) works with both numeric and string inputs - **`test_l2_chain_id_env_var()`** - Verifies environment variable support (`KONA_NODE_L2_CHAIN_ID`) - **`test_l2_chain_id_default()`** - Verifies default behavior (chain ID 10 for Optimism) ### CLI Integration Tests Added (`bin/node/src/cli.rs`) - **`test_cli_l2_chain_id_numeric()`** - Full CLI parsing with numeric chain IDs - **`test_cli_l2_chain_id_string()`** - Full CLI parsing with string chain names - **`test_cli_l2_chain_id_short_flag()`** - Full CLI parsing with short flag - **`test_cli_l2_chain_id_default()`** - Full CLI parsing with default behavior - **`test_cli_l2_chain_id_invalid()`** - Full CLI parsing with invalid inputs ### Documentation Added - **`CHAIN_ID_USAGE.md`** - Comprehensive usage guide showing all supported formats: - Numeric chain IDs: `--l2-chain-id 10` - String chain names: `--l2-chain-id optimism` - Short flag: `-c 10` or `-c optimism` - Environment variable: `KONA_NODE_L2_CHAIN_ID=optimism` ## Usage Examples ```bash # Numeric chain IDs kona-node --l2-chain-id 10 node [args...] # Optimism mainnet kona-node --l2-chain-id 8453 node [args...] # Base mainnet # String chain names kona-node --l2-chain-id optimism node [args...] kona-node --l2-chain-id base node [args...] # Short flag kona-node -c optimism node [args...] # Environment variable export KONA_NODE_L2_CHAIN_ID=optimism kona-node node [args...] ``` ## Testing - All new tests verify the parsing behavior through both direct `GlobalArgs` parsing and full CLI integration - Tests cover positive cases (valid inputs), negative cases (invalid inputs), and edge cases (unknown numeric IDs) - Code compiles successfully with `cargo check --bin kona-node` ## Impact - No breaking changes - the implementation was already using `alloy_chains::Chain` - Significantly improved test coverage for CLI argument parsing - Clear documentation for users on supported chain ID formats - Ensures robust handling of both numeric and string chain identifiers <!-- START COPILOT CODING AGENT TIPS --> --- 💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click [here](https://survey.alchemer.com/s3/8343779/Copilot-Coding-agent) to start the survey. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: refcell <21288394+refcell@users.noreply.github.com> Co-authored-by: refcell <abigger87@gmail.com>
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
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.
Description
Introduces a new contracts-periphery package and initializes the package
with the RetroReceiver contract (meant to receive RetroPGF funds on all
available chains). Reason for introducing a new package is that the
original contracts package should be reserved for in-protocol contracts
while contracts-periphery can be used for any contracts that we want to
write and test, regardless of if they're part of the protocol.