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

feat: add function to obtain eth bridge contracts #260

Merged
merged 8 commits into from
Jun 8, 2023

Conversation

TucksonDev
Copy link
Contributor

First attempt to add these functions (tested on local with several generated networks).

Some notes:

  • I've added the functions to the EthBridger and ERC20Bridger. Please let me know if you think they'd make more sense somewhere else
  • I've created a new interface for the return value of the ERC20Bridger.getGatewayInformation() . That interface looks similar to TokenBridge from dataEntities/network.ts, but as I've added two new fields (l1Gateway and l2Gateway) I didn't want to touch too much the one in the network file.
  • In that interface, l1Gateway and l2Gateway correspond to the information of the gateway passed as parameter, which could be a custom gateway. Not sure about the naming, but please let me know of any good ideas you might have.
  • The logic of the functions should be simple enough, but I might have missed a shortcut somewhere. Please, let me know if that's the case.
  • I've also fixed one logged variable in deployBridge.ts

@cla-bot cla-bot bot added the cla-signed label Mar 10, 2023
src/lib/assetBridger/erc20Bridger.ts Outdated Show resolved Hide resolved
src/lib/assetBridger/erc20Bridger.ts Outdated Show resolved Hide resolved
src/lib/assetBridger/erc20Bridger.ts Outdated Show resolved Hide resolved
src/lib/assetBridger/erc20Bridger.ts Outdated Show resolved Hide resolved
@gzeoneth
Copy link
Member

so I think the only thing you might want to add here is getL1CounterpartGateway and getL2CounterpartGateway. Also note that counterpartGateway is also not part of the token gateway interface, and you should validate the data the gateway return against the other side.

@TucksonDev
Copy link
Contributor Author

Changed logic so only the EthBridge information is retrieved from now.
Moved it to the dataEntities/networks file as having it in the EthBridger file requires an L2Network to be set.

@TucksonDev TucksonDev requested a review from spsjvc May 26, 2023 11:14
@spsjvc spsjvc changed the title Add functions to obtain addresses of the bridge contracts deployed feat: add function to obtain eth bridge contract addresses May 26, 2023
@spsjvc spsjvc changed the title feat: add function to obtain eth bridge contract addresses feat: add function to obtain eth bridge contracts May 31, 2023
* of Arbitrum One network (42161)
*/
describe('Obtain deployed bridge addresses', () => {
it('obtains deployed ETH Bridge addresses', async () => {
Copy link
Member

Choose a reason for hiding this comment

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

I'd move this test to integration tests since we're actually hitting the network and we're not mocking anything

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Test moved

@spsjvc spsjvc requested a review from gzeoneth June 8, 2023 08:49
Copy link
Member

@gzeoneth gzeoneth left a comment

Choose a reason for hiding this comment

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

LGTM

@spsjvc spsjvc merged commit ef76725 into main Jun 8, 2023
@spsjvc spsjvc deleted the get-deployed-bridge-contracts branch June 8, 2023 08:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants