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 Kathy sending from PolygonZkEvm & require MultiProtocolProvider chains to match addresses in MultiProtocolApp #3001

Merged
merged 7 commits into from
Dec 1, 2023

Conversation

tkporter
Copy link
Collaborator

Description

Fixes 2 issues:

  1. Estimates gas in Kathy by explicitly specifying the from address due to this bug with PolygonZkEvm when using a non-zero value eth_estimateGas and eth_call error if a non-zero value is specified without a funded from address 0xPolygonHermez/zkevm-node#2869
  2. Now that we've added neutron & mantapacific as mainnet chains but we don't have helloworld deployments on these chains, Kathy was trying to send to & from these chains. To fix, I changed the constructor of MultiProtocolApp to intersect the multiProvider to only work with the chains specified in addresses

Drive-by changes

n/a

Related issues

n/a

Backward compatibility

ye

Testing

Ran kathy locally successfully & sent from polygonzkevm

Copy link

changeset-bot bot commented Nov 29, 2023

⚠️ No Changeset found

Latest commit: 9caa7f5

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link

codecov bot commented Nov 29, 2023

Codecov Report

Merging #3001 (9caa7f5) into main (6044fb8) will decrease coverage by 67.52%.
Report is 32 commits behind head on main.
The diff coverage is n/a.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #3001       +/-   ##
==========================================
- Coverage   67.51%   0.00%   -67.52%     
==========================================
  Files         101       1      -100     
  Lines        1022      16     -1006     
  Branches      106       0      -106     
==========================================
- Hits          690       0      -690     
+ Misses        292      16      -276     
+ Partials       40       0       -40     
Components Coverage Δ
core ∅ <ø> (∅)
hooks ∅ <ø> (∅)
isms ∅ <ø> (∅)
token ∅ <ø> (∅)
middlewares ∅ <ø> (∅)

@tkporter tkporter enabled auto-merge (squash) November 30, 2023 17:36
@tkporter tkporter changed the title Fix Kathy sending from PolygonZkEvm & intersect MultiProvider in MultiProtocolApp Fix Kathy sending from PolygonZkEvm & require MultiProtocolProvider chains to match addresses in MultiProtocolApp Dec 1, 2023
@tkporter tkporter enabled auto-merge (squash) December 1, 2023 11:16
@tkporter tkporter merged commit e21e020 into main Dec 1, 2023
16 of 23 checks passed
@tkporter tkporter deleted the trevor/fix-kathy-nov-29 branch December 1, 2023 11:16
tkporter added a commit that referenced this pull request Dec 1, 2023
### Description

Deploy Kathy with #3001 

### Drive-by changes

Small fix to deploy kathy successfully - because the relayer chains
doesn't include neutron and kathy will try to create a multiprovider for
all chains the env, we need to use the environment chain names as the
helm `hyperlane.chains` value

### 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
-->
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.

2 participants