-
Notifications
You must be signed in to change notification settings - Fork 429
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: reuse default validators for CLI #2922
Merged
Merged
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
### Description Includes newly deployed Nautilus warps to not enforce gas payments between ### Drive-by changes n/a ### Related issues Related hyperlane-xyz/issues#566 ### Backward compatibility yes ### Testing Deployed
### Description the `debug!` logs either are not occurring very often, or are not displayed at all in the gcp logs <!-- What's included in this PR? --> ### Drive-by changes <!-- Are there any minor or drive-by changes also included? --> ### Related issues <!-- - Fixes #[issue number here] --> ### Backward compatibility <!-- Are these changes backward compatible? Are there any infrastructure implications, e.g. changes that would prohibit deploying older commits using this infra tooling? Yes/No --> ### Testing <!-- What kind of testing have these changes undergone? None/Manual/Unit Tests -->
### Description Getter and setter for the warp route igp <!-- What's included in this PR? --> ### Drive-by changes <!-- Are there any minor or drive-by changes also included? --> ### Related issues - Fixes hyperlane-xyz/issues#571 ### Backward compatibility <!-- Are these changes backward compatible? Are there any infrastructure implications, e.g. changes that would prohibit deploying older commits using this infra tooling? Yes/No --> ### Testing Manual
### Description New relayer gas payment policies for testnet3 and mainnet2, based on these rules: - ~~leave `PJH5QAbxAqrrnSXfH3GHR8icua8CDFZmo97z91xmpvx` in the `GasPaymentEnforcementPolicyType.None` matching list until we move its ownership~~ - don't enforce gas payments: - from the helloworld router on solanadevnet - from solana devnet and mainnet warp routes to anything - with the exception of `EJqwFjvVJSAxH8Ur2PYuMfdvoJeutjmH6GkoEFQ4MdSa`, which now has an igp configured (`HksFWQM1EXJJ5mxo2uZoMfmksXHaNhCunh71NqcQQHZ8`) - from the bsc/bsctestnet routers of all the warp routes - for interchain queries on both testnet3 and mainnet2 ### Related issues - Fixes hyperlane-xyz/issues#570 ### Backward compatibility Yes ### Testing None
### Description Currently, when the validator is not signing, there is just a unhelpful "Could not fetch metadata" log line. This indicates that no quorum could be found at the debug level as well as adding the checkpoint syncer information ### Backward compatibility Yes ### Testing Manual
## Bug 1 Closes #2723 The relayer panic is caused by an overflow, bc of dividing by ~`6.540888459481895e-211`. On my local, the effective rate of indexing starts at `0.61`. ``` {"timestamp":"2023-09-15T09:57:10.746276Z","level":"INFO","fields":{"message":"~~~ blocks_processed: 2508, tip_progression: 2042, elapsed: 757.10340475, old_rate: Some(0.6155037701275111), effective_rate: 0.6155037701275111"},"target":"hyperlane_base::contract_sync::eta_calculator","span":{"domain":"solanadevnet","label":"gas_payments","name":"ContractSync"},"spans":[{"domain":"solanadevnet","label":"gas_payments","name":"ContractSync"}]} ``` But then both the `blocks_processed` and the `tip_progression` are consistently zero, which makes the `new_rate` be zero (https://github.com/hyperlane-xyz/hyperlane-monorepo/blob/eea423ad049acfd15855465792147fb99bc8dd4d/rust/hyperlane-base/src/contract_sync/eta_calculator.rs#L41), and over time it takes over the entire moving average window to make it almost zero, leading to an overflow. 15 mins after that initial log, the effective rate already became `0.00038`. The reason for blocks_processed and tip_progression consistently being zero after the first log is that `eta_calculator.calculate(from, tip)` is always called with the same from and tip although it expects to get the latest values. ### The fix the tip wasn't being set after the sequence_and_tip query here: https://github.com/hyperlane-xyz/hyperlane-monorepo/blob/eea423ad049acfd15855465792147fb99bc8dd4d/rust/hyperlane-base/src/contract_sync/cursor.rs#L565 And then the to and from are calculated based on it: https://github.com/hyperlane-xyz/hyperlane-monorepo/blob/eea423ad049acfd15855465792147fb99bc8dd4d/rust/hyperlane-base/src/contract_sync/cursor.rs#L550 So even though the sync_state internal variables were kept up-to-date, the min(...) would cause the issue. The first PR commit fixes this. ## Bug 2 There was another bug in the eta calculator, caused by it only using `next_block` to approximate synced state, which doesn't apply to sequence indexing. The way the eta calculator is called had to be changed to use abstract measures of sync progression (which could be blocks or sequences). The second PR commit fixes this, afaict.
### Description This PR introduces a feature to warp routes such that LPs can fill transactions on local chains and get a fee for doing this. ### Drive-by changes No. ### Related issues #2690 ### Backward compatibility No ### Testing Added Unit tests. --------- Co-authored-by: Rohan Shrothrium <[email protected]> Co-authored-by: Nam Chu Hoai <[email protected]>
#2758) ### Description - Some small changes to help with debugging sealevel IGP - Turns out the GasPaymentAccount data format onchain is different from the one in `main`. This causes IGP indexing to not work in the wild, but it works in e2e. Because we were planning to change the IGP anyways as we transfer ownership to Zebec, this isn't so bad - so temporarily just not enforcing ZBC warp route Solana -> Nautilus gas payments. In practice, we're still effectively enforcing gas payments because the warp route will pay for gas ### Drive-by changes - Also added the new BSC POSE addy ### Related issues n/a ### Backward compatibility yes ### Testing Deployed, poked around, used cmds
### Description Regular rust version and package updates. ### Drive-by changes None ### Related issues ### Backward compatibility Yes ### Testing Unit Tests
### Description Caps the msg backoff instead of going to insanely large durations ### Backward compatibility Yes ### Testing None
### Description Because incremental merkle tree insertion complexity can change depending on what the index in the merkle tree is, we ran into a situation where the 200k default compute limit wasn't enough when trying to perform a transfer remote on sealevel. Normal cost for index 257, about 158k units: https://explorer.solana.com/tx/4oB9b6PaaKvGMhrQS7XGmi7zBpX3GUW99sbVrSjFwm5AVT96emKF4NH6gn2y9cLMLep8wQJ9LqwJifG5QMpr4BAs Index 255, about 245k units!: https://explorer.solana.com/tx/3v6SbrT58smwnLdnrEFaMXowaEDmo6EpPmKFTUMe9aESo5FTExmJdr2uMH5QJW9hnZTqrB1mfFZe4Vmu75CCAyPs The fix is to simply request more units. Solana tx fees don't actually charge you more if you use more units. Txs have a global cap of 1.4M units -- I'm just requesting 1M here as a balance between having lots of units & not bricking ourselves if the max compute unit amount ever decreases ### Drive-by changes n/a ### Related issues n/a ### Backward compatibility Yes ### Testing I tested a version of this in the UI - I went to the commit where we published 1.5.1, and then published a 1.5.1-beta2 version with this change and moved the UI over to it. Opening this now so we can get this in main and move the UI over to a less sketchy version
### Description * HelloWorld program added to `rust/sealevel/programs/helloworld` * Some changes to the Sealevel tooling * Refactored the old warp-route-specific deploy management into a more generic framework for idempotently deploying and managing router apps in Sealevel * Added helloworld deploy tooling * Added ISM & owner checking to this router deployment tooling * Added `--require-tx-approval` to prevent txs from being called without first prompting * Added a bunch of new commands for creating txs with certain instructions that were needed along the way -- e.g. setting an ISM, deploying a specific multisig ISM (previously only one would be deployed as a part of `core deploy` * Added a command for configuring multisig ISMs, `multisig-ism-message-id configure`, that takes in a JSON file of multisig ISM configs and applies them onchain * A bit of cleanup / refactor - e.g. removed some old commands like `mailbox receive` * Added foreignDeployments into `RouterApp` * Because RouterApp takes in `contractsMap: HyperlaneContractsMap<Factories>,`, which require attached contracts, a new `readonly foreignDeployments: ChainMap<Address> = {},` is added to the constructor * These foreignDeployments are considered in the return value of `remoteChains(chainName: string)`, but not in `chains()` -- this means that `chains()` now concretely means "chains that can be deployed to / interacted with and that there is an entry in `contractsMap` for, and `remoteChains(chainName: string)` returns any and all remote chains, regardless of whether they can be deployed to / interacted with * Added complete ISM support to the HyperlaneRouterChecker * when checking the ISM, if there's not a match and the ISM is a config, then the ISM will be deployed * Also added RouterViolation, before it'd just throw if there was a violation * Updated the Helloworld, IGP, and core tooling to work when AEE deployments are also configured * Moved to Routing ISM -> Aggregation ISM -> Merkle / Message ID multisig setup Some things to note: * atm there are a few places that have a TODO saying to remove something after some multisig txs are executed, I plan to revisit these after we get some sigs * I've deployed the mainnet sealevel version of helloworld, but haven't been able to enroll it in the EVM chains yet. Waiting for some multisig activity here ### Drive-by changes ### Related issues #2502 ### Backward compatibility I believe it should all be backward compatible ### Testing Deployed, ran checkers, etc
### Description - Adapter fixes from further testing of multi-protocol Kathy - Add router address for hyperlane context helloworld - Improve stat collection to handle cross-protocol msgs - Re-enable actual env config for Kathy ### Drive-by changes - Fix for transaction hash checking in utils ### Related issues #2503 ### Backward compatibility Yes ### Testing Ran Kathy manually using `getCoreConfigStub` and forced stat output Tested live in cloud with help from @tkporter --------- Co-authored-by: Trevor Porter <[email protected]>
### Description <!-- What's included in this PR? --> ### Drive-by changes <!-- Are there any minor or drive-by changes also included? --> ### Related issues <!-- - Fixes #[issue number here] --> ### Backward compatibility <!-- Are these changes backward compatible? Are there any infrastructure implications, e.g. changes that would prohibit deploying older commits using this infra tooling? Yes/No --> ### Testing <!-- What kind of testing have these changes undergone? None/Manual/Unit Tests -->
Per discussion in standup yesterday
### Description - Rpc urls are super flaky, stopping the validators and relayer support - Keeping proteustestnet artifacts / chain metadata for now ### Drive-by changes <!-- Are there any minor or drive-by changes also included? --> ### Related issues <!-- - Fixes #[issue number here] --> ### Backward compatibility <!-- Are these changes backward compatible? Are there any infrastructure implications, e.g. changes that would prohibit deploying older commits using this infra tooling? Yes/No --> ### Testing <!-- What kind of testing have these changes undergone? None/Manual/Unit Tests -->
I guess this is goodbye.
### Description - Create new CLI package - Upgrade prettier, typescript, and lint packages - Update TSConfig output target to ES2020 - Migrate code from hyp-deploy repo to `typescript/cli/src` ### Related issues Fixes #2566 Fixes #2786 ### Backward compatibility Yes ### Testing Tested all commands locally Tested core deployments with and without pre-existing addresses (artifacts) Created e2e tests for deploying core, warp, and sending messages --------- Co-authored-by: Nam Chu Hoai <[email protected]>
Improve output format of chains list command
### Description - Support multiple origins in PI deployments - Simplify ci test with single core deploy command - Improve output format of chains list command ### Related issues Fixes #2813 ### Testing Tested locally and with e2e test
Update codeowners Use consistent eslint version
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## v3 #2922 +/- ##
=======================================
Coverage 67.94% 67.94%
=======================================
Files 99 99
Lines 1017 1017
Branches 106 106
=======================================
Hits 691 691
Misses 286 286
Partials 40 40
|
jmrossy
approved these changes
Nov 15, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm
Merged
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
Drive-by changes
multisig-ism.yaml
and deploying messageIdMultisig for all multisig configs provided indiscriminately. Also, added a note regarding this in theexample/multisig-ism.yaml
file. (otherwise we end up usingMultisigIsmConfig
andMultisigConfig
interchangeably like in thebuildIgpConfig
which leads to confusion)NOTE: this doesn't affect our inability to not deploy the same multisig with different types for different remote chains because the config provided is origin specific, eg, you cannot use goerli multisig as messageId for arbitrumsepolia and for scrollsepolia as merkleRoot.
buildMultisigIsmConfigs
to sdk as an effort to dedupe the two variations in infra and cli.Related issues
Backward compatibility
Yes
Testing
Manual