feat: ergonomic IGP configuration in CLI #4635
Open
Conversation
This file contains 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
🦋 Changeset detectedLatest commit: 670b211 The changes in this PR will be included in the next version bump. This PR includes changesets to release 9 packages
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 |
paulbalaji
requested review from
tkporter,
Mo-Hussain,
jmrossy,
yorhodes and
ltyu
as code owners
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4635 +/- ##
=======================================
Coverage 73.89% 73.89%
=======================================
Files 100 100
Lines 1421 1421
Branches 180 180
=======================================
Hits 1050 1050
Misses 350 350
Partials 21 21
|
xeno097
requested changes
Oct 8, 2024
Comment on lines
+105
to
+109
| local: ChainName, | ||
| remote: ChainName, | ||
| tokenPrices: ChainMap<string>, | ||
| exchangeRateMarginPct: number, | ||
| decimals: { local: number; remote: number }, |
There was a problem hiding this comment.
Just a personal preference but I think it would be better to convert this list of params to a single object so that when this function is used we can directly see what each value is just by looking at the field names
There was a problem hiding this comment.
this is also my preference wherever possible, I just didn't want to break the current tradition of individual args vs one object of args that we've got in utils. definitely down to make it an object if people are ok with it
Comment on lines
-190
to
328
| ); | ||
| const isTestnet = context.chainMetadata[localChain].isTestnet; | ||
| const remoteChains = await runMultiChainSelectionStep( | ||
| objFilter( | ||
| context.chainMetadata, | ||
| (_, metadata): metadata is ChainMetadata => | ||
| metadata.name !== localChain, | ||
| ), | ||
| 'Select remote destination chains for IGP hook', | ||
| 1, | ||
| isTestnet ? 'testnet' : 'mainnet', | ||
| ); | ||
|
|
||
| // Only need to set overhead for remote chains | ||
| const overhead: ChainMap<number> = {}; | ||
| for (const chain of remoteChains) { | ||
| overhead[chain] = parseInt( | ||
| await input({ | ||
| message: `Enter overhead for ${chain} (eg 75000) for IGP hook`, | ||
| message: `Enter overhead for ${chain} (e.g., 75000) for IGP hook`, | ||
| default: '75000', | ||
| }), | ||
| ); | ||
| overheads[chain] = overhead; | ||
| } | ||
|
|
||
| // Restrict metadata to only the chains that are part of the IGP hook | ||
| const filteredMetadata = objFilter( | ||
| context.chainMetadata, | ||
| (_, metadata): metadata is ChainMetadata => | ||
| remoteChains.includes(metadata.name) || metadata.name === localChain, | ||
| ); | ||
|
|
||
| // Fetch prices for all chains in the IGP hook | ||
| // For testnet, hardcode prices to 10 USD | ||
| // For mainnet, fetch prices from Coingecko | ||
| // If mainnet prices are not found, user will be prompted to enter them | ||
| let fetchedPrices: ChainMap<string | undefined> = {}; | ||
| if (isTestnet) { | ||
| // Get the prices of the remote chains' tokens from Coingecko | ||
| logBlue(`Hardcoding all gas token prices to 10 USD for testnet...`); | ||
| fetchedPrices = objMap(filteredMetadata, () => '10'); | ||
| } else { | ||
| // Get the prices of the remote chains' tokens from Coingecko | ||
| logBlue(`Getting gas token prices for all chains from Coingecko...`); | ||
| fetchedPrices = await getCoingeckoTokenPrices(filteredMetadata); | ||
| } | ||
|
|
||
| const mpp = new MultiProtocolProvider(context.chainMetadata); | ||
| const prices: ChainMap<ChainGasOracleParams> = {}; | ||
|
|
||
| for (const chain of Object.keys(filteredMetadata)) { | ||
| const gasPrice = await getGasPrice(mpp, chain); | ||
| logBlue(`Gas price for ${chain} is ${gasPrice.amount}`); | ||
|
|
||
| // if the price is not found on Coingecko, prompt user to enter it | ||
| let tokenPrice = fetchedPrices[chain]; | ||
| if (!tokenPrice) { | ||
| tokenPrice = await input({ | ||
| message: `Enter the price of ${chain}'s token in USD`, | ||
| }); | ||
| } else { | ||
| logBlue(`Gas token price for ${chain} is $${tokenPrice}`); | ||
| } | ||
|
|
||
| const decimals = context.chainMetadata[chain].nativeToken?.decimals; | ||
| if (!decimals) { | ||
| throw new Error(`No decimals found in metadata for ${chain}`); | ||
| } | ||
| prices[chain] = { | ||
| gasPrice, | ||
| nativeToken: { price: tokenPrice, decimals }, | ||
| }; | ||
| } | ||
|
|
||
| // Allow user to enter a IGP exchange rate margin in % | ||
| const exchangeRateMarginPct = parseInt( | ||
| await input({ | ||
| message: `Enter IGP margin percentage (e.g. 10 for 10%)`, | ||
| default: '10', | ||
| }), | ||
| ); | ||
|
|
||
| const oracleConfig = getLocalStorageGasOracleConfig( | ||
| localChain, | ||
| prices, | ||
| exchangeRateMarginPct, | ||
| ); | ||
|
|
||
| return { | ||
| type: HookType.INTERCHAIN_GAS_PAYMASTER, | ||
| beneficiary, | ||
| owner, | ||
| oracleKey, | ||
| overhead: overheads, | ||
| oracleConfig: {}, | ||
| overhead, | ||
| oracleConfig, | ||
| }; | ||
| }, |
There was a problem hiding this comment.
My only complaint here is that the function body is bit long maybe some code block might be exported into their own functions. Examples:
This could go into its own function named something like getIgpOwner
const unnormalizedOwner =
!advanced && context.signer
? await context.signer.getAddress()
: await detectAndConfirmOrPrompt(
async () => context.signer?.getAddress(),
'For Interchain Gas Paymaster, enter',
'owner address',
'signer',
);This could be put inside a getGasPriceByChain function or something similar
const prices: ChainMap<ChainGasOracleParams> = {};
for (const chain of Object.keys(filteredMetadata)) {
const gasPrice = await getGasPrice(mpp, chain);
logBlue(`Gas price for ${chain} is ${gasPrice.amount}`);
// if the price is not found on Coingecko, prompt user to enter it
let tokenPrice = fetchedPrices[chain];
if (!tokenPrice) {
tokenPrice = await input({
message: `Enter the price of ${chain}'s token in USD`,
});
} else {
logBlue(`Gas token price for ${chain} is $${tokenPrice}`);
}
const decimals = context.chainMetadata[chain].nativeToken?.decimals;
if (!decimals) {
throw new Error(`No decimals found in metadata for ${chain}`);
}
prices[chain] = {
gasPrice,
nativeToken: { price: tokenPrice, decimals },
};
}There was a problem hiding this comment.
hmm i'll have a play around
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
Re-adding the ability to generate IGP hook configs using the CLI, but repurposing logic found in infra to make the configuration experience more ergonomic. Logic still behind the
--advancedflag.We will use metadata in registry to:
gasPrice+tokenExchangeRatefor youNote that it still sets
overheadto some preexisting default.Core config is valid, writing to file ./configs/core-config.yaml: owner: "0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266" defaultIsm: type: trustedRelayerIsm relayer: "0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266" defaultHook: type: aggregationHook hooks: - type: merkleTreeHook - owner: "0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266" type: interchainGasPaymaster beneficiary: "0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266" oracleKey: "0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266" overhead: alephzero: 75000 ancient8: 75000 oracleConfig: alephzero: gasPrice: "40000000000" tokenExchangeRate: "129868" ancient8: gasPrice: "1000253" tokenExchangeRate: "778100236" requiredHook: owner: "0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266" type: protocolFee beneficiary: "0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266" maxProtocolFee: "1000000000000000000" protocolFee: "0" ✅ Successfully created new core deployment config.Drive-by changes
Moving reusable infra logic into the SDK, and refactoring CLI+Infra to reuse the underlying logic. For example:
Related issues
Most recently, hyperlane-xyz/hyperlane-registry#236 (comment). But there have been numerous occasions where it would be nice for users to be self-sufficient in configuring and deploying an IGP hook for their PI deployments/relayer.
Backward compatibility
yes
Testing
hyperlane core init --advanced