Skip to content

Commit

Permalink
Fix Kathy sending from PolygonZkEvm & require MultiProtocolProvider c…
Browse files Browse the repository at this point in the history
…hains to match addresses in MultiProtocolApp (#3001)

### 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
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
  • Loading branch information
tkporter authored Dec 1, 2023
1 parent 852b8d8 commit e21e020
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 5 deletions.
10 changes: 9 additions & 1 deletion typescript/helloworld/src/multiProtocolApp/evmAdapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ export class EvmHelloWorldAdapter
destination: ChainName,
message: string,
value: string,
sender: Address,
): Promise<EthersV5Transaction> {
const contract = this.getConnectedContract();
const toDomain = this.multiProvider.getDomainId(destination);
Expand All @@ -44,7 +45,14 @@ export class EvmHelloWorldAdapter
const estimated = await contract.estimateGas.sendHelloWorld(
toDomain,
message,
{ ...transactionOverrides, value: BigNumber.from(value).add(quote) },
{
...transactionOverrides,
// Some networks, like PolygonZkEvm, require a `from` address
// with funds to be specified when estimating gas for a transaction
// that provides non-zero `value`.
from: sender,
value: BigNumber.from(value).add(quote),
},
);
const gasLimit = estimated.mul(12).div(10);

Expand Down
2 changes: 1 addition & 1 deletion typescript/infra/scripts/helloworld/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ export async function getHelloWorldMultiProtocolApp(
}),
);
const app = new HelloMultiProtocolApp(
multiProtocolProvider,
multiProtocolProvider.intersect(Object.keys(routersAndMailboxes)).result,
routersAndMailboxes,
);

Expand Down
8 changes: 7 additions & 1 deletion typescript/sdk/src/app/MultiProtocolApp.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,13 @@ describe('MultiProtocolApp', () => {
describe('constructs', () => {
const multiProvider = new MultiProtocolProvider();
it('creates an app class and gleans types from generic', async () => {
const app = new TestMultiProtocolApp(multiProvider, {});
const addresses = {
ethereum: {},
};
const app = new TestMultiProtocolApp(
multiProvider.intersect(Object.keys(addresses)).result,
addresses,
);
expect(app).to.be.instanceOf(MultiProtocolApp);
expect(app.adapter(Chains.ethereum).protocol).to.eql(
ProtocolType.Ethereum,
Expand Down
17 changes: 17 additions & 0 deletions typescript/sdk/src/app/MultiProtocolApp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
ProtocolType,
objMap,
promiseObjAll,
symmetricDifference,
} from '@hyperlane-xyz/utils';

import { ChainMetadata } from '../metadata/chainMetadataTypes';
Expand Down Expand Up @@ -120,6 +121,22 @@ export abstract class MultiProtocolApp<
public readonly addresses: ChainMap<ContractAddrs>,
public readonly logger = debug('hyperlane:MultiProtocolApp'),
) {
const multiProviderChains = multiProvider.getKnownChainNames();
const addressesChains = Object.keys(addresses);
const setDifference = symmetricDifference(
new Set(multiProviderChains),
new Set(addressesChains),
);
if (setDifference.size > 0) {
throw new Error(
`MultiProtocolProvider and addresses must have the same chains. Provider chains: ${multiProviderChains.join(
', ',
)}. Addresses chains: ${addressesChains.join(
', ',
)}. Difference: ${Array.from(setDifference)}`,
);
}

super(multiProvider.metadata);
}

Expand Down
8 changes: 6 additions & 2 deletions typescript/sdk/src/router/MultiProtocolRouterApps.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,13 @@ describe('MultiProtocolRouterApp', () => {
describe('constructs', () => {
const multiProvider = new MultiProtocolProvider<RouterAddress>();
it('creates an app class', async () => {
const app = new MultiProtocolRouterApp(multiProvider, {
const addresses = {
ethereum: { router: ethers.constants.AddressZero },
});
};
const app = new MultiProtocolRouterApp(
multiProvider.intersect(Object.keys(addresses)).result,
addresses,
);
expect(app).to.be.instanceOf(MultiProtocolRouterApp);
const ethAdapter = app.adapter(Chains.ethereum);
expect(ethAdapter).to.be.instanceOf(EvmRouterAdapter);
Expand Down

0 comments on commit e21e020

Please sign in to comment.