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: reuse default validators for CLI #2922

Merged
merged 51 commits into from
Nov 15, 2023
Merged

Conversation

aroralanuk
Copy link
Contributor

Description

  • if the default multisig config contains the config for the chain we want to deploy on and we've not explicitiy provided a config, the CLI will use the default config

Drive-by changes

  • ignoring the ismType in multisig-ism.yaml and deploying messageIdMultisig for all multisig configs provided indiscriminately. Also, added a note regarding this in the example/multisig-ism.yaml file. (otherwise we end up using MultisigIsmConfig and MultisigConfig interchangeably like in the buildIgpConfig 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.

  • moved buildMultisigIsmConfigs to sdk as an effort to dedupe the two variations in infra and cli.

Related issues

Backward compatibility

Yes

Testing

Manual

tkporter and others added 30 commits September 14, 2023 08:41
### 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
Copy link

codecov bot commented Nov 15, 2023

Codecov Report

Merging #2922 (8f6bd86) into v3 (00a91f8) will not change coverage.
The diff coverage is n/a.

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           
Components Coverage Δ
core 50.00% <ø> (ø)
hooks 71.42% <ø> (ø)
isms 70.50% <ø> (ø)
token 58.41% <ø> (ø)
middlewares 81.46% <ø> (ø)

Copy link
Contributor

@jmrossy jmrossy left a comment

Choose a reason for hiding this comment

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

Lgtm

@aroralanuk aroralanuk merged commit 69d87c9 into v3 Nov 15, 2023
@aroralanuk aroralanuk deleted the kunal/reuse-default-validators-cli branch November 15, 2023 22:33
@yorhodes yorhodes mentioned this pull request Nov 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

8 participants