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

fix(configs): Refactor contracts config #3660

Open
wants to merge 25 commits into
base: deniallugo-multilayer-client
Choose a base branch
from

Conversation

Deniallugo
Copy link
Contributor

@Deniallugo Deniallugo commented Feb 28, 2025

What ❔

Refactor contracts config and moving it to framework. Also it support both layers Gateway and L1, allowing to automatically decide what layer supposed to be used now.

Why ❔

In the future the contracts config will be loaded from Settlement Layer. It's the preparation, where no components depends on the contracts config directly.

Is this a breaking change?

  • Yes
  • No

Operational changes

Checklist

  • PR title corresponds to the body of PR (we generate changelog entries from PRs).
  • Tests for the changes have been added / updated.
  • Documentation comments have been added / updated.
  • Code has been formatted via zkstack dev fmt and zkstack dev lint.

@Deniallugo Deniallugo changed the title Deniallugo contracts config refactoring fix(configs): Refactor contracts config Mar 3, 2025
Signed-off-by: Danil <[email protected]>
@Deniallugo Deniallugo force-pushed the deniallugo-contracts-config-refactoring branch from d30c61f to 2cd7b6b Compare March 3, 2025 19:06
Signed-off-by: Danil <[email protected]>
@Deniallugo Deniallugo force-pushed the deniallugo-contracts-config-refactoring branch from 0fd3fd4 to 62d7129 Compare March 3, 2025 20:49
Signed-off-by: Danil <[email protected]>
Signed-off-by: Danil <[email protected]>
@Deniallugo Deniallugo force-pushed the deniallugo-contracts-config-refactoring branch from 7247fa0 to b0c9be8 Compare March 4, 2025 10:03
Signed-off-by: Danil <[email protected]>
@Deniallugo Deniallugo force-pushed the deniallugo-contracts-config-refactoring branch from c39056e to 1d898b0 Compare March 4, 2025 10:45
@Deniallugo Deniallugo force-pushed the deniallugo-contracts-config-refactoring branch from 174f7aa to cf094e2 Compare March 4, 2025 11:13
Signed-off-by: Danil <[email protected]>
@Deniallugo Deniallugo force-pushed the deniallugo-contracts-config-refactoring branch from af0626e to 0609c50 Compare March 4, 2025 12:40
@Deniallugo Deniallugo force-pushed the deniallugo-contracts-config-refactoring branch from fdc561e to 6638eee Compare March 4, 2025 14:47
Signed-off-by: Danil <[email protected]>
Signed-off-by: Danil <[email protected]>
Signed-off-by: Danil <[email protected]>
Signed-off-by: Danil <[email protected]>
Signed-off-by: Danil <[email protected]>
Signed-off-by: Danil <[email protected]>
@Deniallugo Deniallugo force-pushed the deniallugo-contracts-config-refactoring branch from 23f33d7 to 322346b Compare March 5, 2025 18:19
## What ❔

<!-- What are the changes this PR brings about? -->
<!-- Example: This PR adds a PR template to the repo. -->
<!-- (For bigger PRs adding more context is appreciated) -->

## Why ❔

<!-- Why are these changes done? What goal do they contribute to? What
are the principles behind them? -->
<!-- The `Why` has to be clear to non-Matter Labs entities running their
own ZK Chain -->
<!-- Example: PR templates ensure PR reviewers, observers, and future
iterators are in context about the evolution of repos. -->

## Is this a breaking change?
- [ ] Yes
- [ ] No

## Operational changes
<!-- Any config changes? Any new flags? Any changes to any scripts? -->
<!-- Please add anything that non-Matter Labs entities running their own
ZK Chain may need to know -->

## Checklist

<!-- Check your PR fulfills the following items. -->
<!-- For draft PRs check the boxes as you complete them. -->

- [ ] PR title corresponds to the body of PR (we generate changelog
entries from PRs).
- [ ] Tests for the changes have been added / updated.
- [ ] Documentation comments have been added / updated.
- [ ] Code has been formatted via `zkstack dev fmt` and `zkstack dev
lint`.

---------

Signed-off-by: Danil <[email protected]>
#[derive(Debug, Clone)]
pub struct SettlementLayerContracts {
l1_contracts: ChainSpecificContracts,
gateway_contracts: Option<ChainSpecificContracts>,
Copy link
Contributor

Choose a reason for hiding this comment

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

l2Contracts overlaps, no? In gateway_contracts and l1_contracts

diamond_proxy_contract_address: self.contracts_config.diamond_proxy_addr,
chain_admin_contract_address: Some(self.contracts_config.chain_admin_addr),
diamond_proxy_contract_address: input
.contracts_resource
Copy link
Contributor

Choose a reason for hiding this comment

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

here it was changed from L1 to GW, and for the chainAdmin as well


#[derive(Debug, FromContext)]
#[context(crate = crate)]
pub struct Input {
pub master_pool: PoolResource<MasterPool>,
pub main_node_client: MainNodeClientResource,
pub l1_client: EthInterfaceResource,
pub gateway_client: Option<GatewayEthInterfaceResource>,
pub gateway_client: UniversalClientResource,
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't it be settlment_layer_client? i.e. it is for l1 or gw client, no?

@@ -72,16 +69,6 @@ impl WiringLayer for QueryEthClientLayer {
} else {
None
},
Copy link
Contributor

@kelemeno kelemeno Mar 7, 2025

Choose a reason for hiding this comment

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

why did you get rid of this? We need to watch the GW for EthWatch component. Or did you add it back somewhere else?

We need to watch both L1 and GW

Copy link
Contributor

Choose a reason for hiding this comment

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

ah its in query_gateway_client I think

}

#[async_trait::async_trait]
impl WiringLayer for GatewayClientLayer {
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably rename to SLClientLayer? If it is a GatewayClient it cannot be an L1 client

Signed-off-by: Danil <[email protected]>
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.

3 participants