Conversation
|
xeno097
left a comment
xeno097
left a comment
There was a problem hiding this comment.
I think that maybe in the future we could refactor this a bit in case we have another chain with specific requirements so that we can reuse the same function that build core confgis normally but parametrizing the chains to connect as comparing the getEdenCoreConfig with the function used by default for generating the config, the only thing that really changes is that one uses supportedChainNames while the other uses EDEN_CONNECTED_CHAINS
agreed! one day i'd like to take on these two and do a little config refactoring but alas for the future: |
paulbalaji
requested review from
Mo-Hussain,
Xaroz,
ameten,
antigremlin,
ltyu,
yjamin and
yorhodes
as code owners
📝 WalkthroughWalkthroughAdds full support for a new mainnet chain "eden": chain registry entries, verification artifacts, IGP/Core composition, agent/validator flags, balances/gas/token prices, Docker tag bump, and address updates in Rust configs. Tests adjusted to exclude eden where appropriate. Changes
Sequence Diagram(s)sequenceDiagram
participant Owner
participant EdenModule as eden.ts
participant IGP as InterchainGasPaymaster
participant Core as CoreConfig
participant Agents as Agents/Validators
Owner->>EdenModule: getEdenIgpConfig(owner, storageGasOracleConfig)
EdenModule-->>IGP: build per-chain oracle & overhead config
Owner->>EdenModule: getEdenCoreConfig(owner, igpConfig)
EdenModule-->>Core: assemble ISMs, hooks, routing (merkle, messageId, routing)
Core-->>Agents: export core config for validator/relayer/scraper wiring
Agents->>IGP: wire beneficiary/overrides (runtime)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@typescript/infra/config/environments/mainnet3/core.ts`:
- Around line 43-51: The current originMultisigs creation filters out 'eden'
globally which prevents Celestia from receiving Eden entries; update the filter
chain so 'eden' is only excluded when local !== 'celestia' (i.e., allow 'eden'
when local === 'celestia') while keeping the other filters (no reflexivity and
excluding 'forma'); modify the predicate in the supportedChainNames .filter that
references 'eden' (used when building originMultisigs and relying on
defaultMultisigConfigs) to check local and include eden only for celestia.
In `@typescript/infra/config/environments/mainnet3/eden.ts`:
- Around line 125-127: The thrown error message doesn't match the checked
property: update the error text to reflect the actual validation of owner.owner
(e.g., "owner.owner must be a string" or "owner must be a string") so it's
accurate and unambiguous; modify the throw in the block that contains the
owner.owner type check to use the corrected message.
♻️ Duplicate comments (2)
.registryrc (1)
1-1: Confirm the registry hash points to the Eden update.
Just make sure this commit hash lines up with the registry change you expect for Eden metadata.typescript/infra/config/environments/mainnet3/eden.ts (1)
62-71: Add validation for the celestia multisig config existence.Right now ye just reach into
defaultMultisigConfigs['celestia']like it's guaranteed to be there. But if that config goes missin' for some reason, ye'll end up withundefinedpassin' through and causin' trouble downstream - nobody wants that kind of ogre-sized headache.Also, 'celestia' is hardcoded here while
EDEN_CONNECTED_CHAINSexists separately. If more chains get added to that array, this logic won't automatically pick 'em up.🛠️ Suggested fix
export function getEdenCoreConfig( owner: OwnableConfig, igpConfig: IgpConfig, ): CoreConfig { const celestiaMultisig = defaultMultisigConfigs['celestia']; + assert(celestiaMultisig, 'Missing multisig config for celestia'); const merkleRoot = multisigConfigToIsmConfig(
🧹 Nitpick comments (1)
typescript/infra/config/environments/mainnet3/eden.ts (1)
28-31: Replace weakanytyping with the properAllStorageGasOracleConfigstype.The
Record<string, any>parameter is a bit of a swamp—it's wide open and anything can slip through. The codebase already has a proper type for this,AllStorageGasOracleConfigs, defined right there in the gas-oracle config. Your neighbor igp.ts in the same folder is using it already.Import it alongside
getOverheadand give the parameter proper type coverage. Ye'll get better compile-time guarantees that way.
🐳 Monorepo Docker Image Built SuccessfullyImage Tags: |
🦀 Rust Agent Docker Image Built SuccessfullyImage Tags: |
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7863 +/- ##
=======================================
Coverage 77.02% 77.02%
=======================================
Files 117 117
Lines 2651 2651
Branches 244 244
=======================================
Hits 2042 2042
Misses 593 593
Partials 16 16
🚀 New features to boost your workflow:
|
2 participants
Description
feat: deploy to eden
Drive-by changes
Related issues
Backward compatibility
Testing
Summary by CodeRabbit
New Features
Chores
Tests
✏️ Tip: You can customize this high-level summary in your review settings.