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

Refactor IGP related code out of HyperlaneCore #1907

Merged
merged 53 commits into from
Mar 23, 2023
Merged

Refactor IGP related code out of HyperlaneCore #1907

merged 53 commits into from
Mar 23, 2023

Conversation

asaj
Copy link
Contributor

@asaj asaj commented Mar 7, 2023

Description

A number of changes intended to make the SDK more easily consumed by hyperlane-deploy

  • Splits the IGP related code out of HyperlaneCore* into HyperlaneIgp*
  • Moves agent config building from infra to SDK so that it can be accessed by hyperlane-deploy
  • Modifies CoreConfig to specify a ChainMap (i.e. specify everything that you need to know to deploy to a chain in that chain's CoreConfig)
  • Removes core.extendWithConnectionClientConfig
  • Moves common logic into HyperlaneAppGovernor to be shared between HyperlaneCoreGovernor and HyperlaneIgpGovernor
  • Adds TestRecipient contract addresses to the SDK addresses for consumption by hyperlane-deploy
  • Allow buildContracts to build when address keys are a superset of factory keys (via filterContracts). This is useful as it allows us to just throw all the addresses at a HyperlaneApp constructor without needing to remove the ones that aren't relevant
  • Removes InterchainGasCalculator, the logic we want to keep now lives in HyperlaneIgp
  • Allows chains to be added to a MultiProvider

Drive-by changes

  • Merges the infra govern script into the check script (to dedupe)
  • Some minor renaming where "Core" was being used liberally
  • Default to using whatever is configured in owners.ts in the router configs for infra (for ICA, IQS, HelloWorld, and LL deployment)

Related issues

  • Fixes #[issue number here]

Backward compatibility

Are these changes backward compatible?

No, existing consumers of the SDK may need to adjust.

Are there any infrastructure implications, e.g. changes that would prohibit deploying older commits using this infra tooling?

None

Testing

What kind of testing have these changes undergone?

Manual / Unit Tests

@asaj asaj changed the base branch from main to asaj/sdk-refactor March 7, 2023 22:42
Base automatically changed from asaj/sdk-refactor to main March 10, 2023 17:32
tkporter added a commit that referenced this pull request Mar 13, 2023
### Description

* Adds DefaultIsmInterchainGasPaymaster to the checker and govern logic
* I'm aware this conflicts with
#1907 -- I wrote
this mostly so I could propose new multisig txs to update the overhead
gas amounts on mainnet. Happy to either merge this before or after
#1907, whatever
works easiest

### Drive-by changes

* Some fixes in the wake of the sdk refactor, e.g.:
* if some tx params like `maxPriorityFeePerGas` or `maxFeePerGas` were
specified when we estimate gas, it can fail if the balance of the sender
is insufficient. For cases where we want to estimate gas from the owner
multisig which may not have native tokens, this caused issues.
* We should've been passing the `from` address into the
MultiProvider.estimateGas fn as a separate param

### Related issues

n/a

### Backward compatibility

_Are these changes backward compatible?_

Yes

_Are there any infrastructure implications, e.g. changes that would
prohibit deploying older commits using this infra tooling?_

None


### Testing

_What kind of testing have these changes undergone?_

Ran & proposed some txs on mainnet to update the destination gas
overheads
@asaj asaj changed the title Refactor IGP related code out of HyperlaneCore* Refactor IGP related code out of HyperlaneCore Mar 14, 2023
@asaj asaj requested review from jmrossy and yorhodes and removed request for nambrot, mattiekat and tkporter March 20, 2023 14:22
typescript/infra/scripts/deploy.ts Outdated Show resolved Hide resolved
@asaj asaj enabled auto-merge (squash) March 23, 2023 18:27
@asaj asaj merged commit 97d1757 into main Mar 23, 2023
@asaj asaj deleted the asaj/igp-refactor branch March 23, 2023 19:06
yorhodes pushed a commit that referenced this pull request Mar 23, 2023
change history!!!

A number of changes intended to make the SDK more easily consumed by
hyperlane-deploy

- Splits the IGP related code out of HyperlaneCore* into HyperlaneIgp*
- Moves agent config building from infra to SDK so that it can be
accessed by hyperlane-deploy
- Modifies CoreConfig to specify a ChainMap<MultisigIsmConfig> (i.e.
specify everything that you need to know to deploy to a chain in that
chain's CoreConfig)
- Removes core.extendWithConnectionClientConfig
- Moves common logic into HyperlaneAppGovernor to be shared between
HyperlaneCoreGovernor and HyperlaneIgpGovernor
- Adds TestRecipient contract addresses to the SDK addresses for
consumption by hyperlane-deploy
- Allow buildContracts to build when address keys are a superset of
factory keys (via filterContracts). This is useful as it allows us to
just throw all the addresses at a HyperlaneApp constructor without
needing to remove the ones that aren't relevant
- Removes InterchainGasCalculator, the logic we want to keep now lives
in HyperlaneIgp
- Allows chains to be added to a MultiProvider

- Merges the infra govern script into the check script (to dedupe)
- Some minor renaming where "Core" was being used liberally
- Default to using whatever is configured in `owners.ts` in the router
configs for infra (for ICA, IQS, HelloWorld, and LL deployment)

- Fixes #[issue number here]

_Are these changes backward compatible?_

No, existing consumers of the SDK may need to adjust.

_Are there any infrastructure implications, e.g. changes that would
prohibit deploying older commits using this infra tooling?_

None

_What kind of testing have these changes undergone?_

Manual / Unit Tests
@asaj asaj mentioned this pull request Mar 24, 2023
yorhodes added a commit that referenced this pull request Mar 24, 2023
Changed in #1907
@yorhodes yorhodes mentioned this pull request Mar 24, 2023
yorhodes added a commit that referenced this pull request Mar 24, 2023
Changed in #1907

### Testing

Fork tests
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