feat: add reverse look up for contract info from address #155

chrismaree · 2022-05-10T09:28:47Z

adds a helper function to find info about a deployed contract from it's address.
refines getDeployedAddress to let you ignore the network name when fetching a SpokePool address.

Signed-off-by: chrismaree <christopher.maree@gmail.com>

daywiss · 2022-05-10T19:39:42Z

src/DeploymentUtils.ts

+    try {
+      // If this does not match then try search on a Regex on SpokePool to let the caller exclude the chain name.
+      for (const _contractName of Object.keys(deployments[networkId.toString()]))
+        if (/.*_SpokePool/.test(_contractName)) return deployments[networkId.toString()][_contractName].address;


this could error because of a typo, and then the user ends up getting spoke pool address when maybe they meant somethign completely different, which could be bad.

the only way that would happen is if you added SpokePool to the search. this regex will ONLY pass if *_SpokePool is set. you are right though that you could do something silly like HubPool_SpokePool which would return the spoke pool on that chain.

however, I have modified this to export the name of the contract as SpokePool on each respective chain to avoid needing this workaround alltogether.

I see, i missed that check

daywiss · 2022-05-10T19:41:27Z

src/DeploymentUtils.ts

+export function getDeployedBlockNumber(contractName: string, networkId: number): number {
  try {
-    return (deployments as any)[networkId.toString()][contractName].blockNumber;
+    return deployments[networkId.toString()][contractName].blockNumber;


this should have the same behavior as getDeployedAddress. imo we might just want a way to look up the correct contract name, then you dont have to modify thsese 2 functions at all

fixed. see above comments

daywiss · 2022-05-10T19:44:23Z

src/DeploymentUtils.ts

 }
+
+// Returns the chainId and contract name for a given contract address.
+export function getContractInfoFromAddress(contractAddress: string): { chainId: Number; contractName: string } {


it looks like this solves my issues since im usually given the contract address and chain ( at least for spoke pools). It would be a problem if hubpools also had different names, but they dont currently.

ye, this method assumes that the deployment addresses on each chain are unique enabling you to go backwards from a known address to the contract name and chainId.

Does the PolygonTokenBridger break this? It has the same address on multiple chains

Ye that contract would not work with this method but that's ok. This method is only used in the bots to enhance logs and the bots never directly call those polygon contracts so it won't be an issue.

I'll add an error that is thrown if multiple addresses are found to prevent any confusion

Gotcha. Makes sense.

mrice32

One comment!

mrice32 · 2022-05-10T22:12:34Z

src/DeploymentUtils.ts

  } catch (_) {
+    try {
+      // If this does not match then try search on a Regex on SpokePool to let the caller exclude the chain name.
+      for (const _contractName of Object.keys(deployments[networkId.toString()]))


I think this is confusing. Can we just make aliases in the generation? Aka save an identical deployment under a different name using deployments.save?

Something like:

const spokePoolDeployment = await deploy("Optimism_SpokePool", { from: deployer, log: true, skipIfAlreadyDeployed: true, args: [ hubPool.address, // Set hub pool as cross domain admin since it delegatecalls the Optimism_Adapter logic. hubPool.address, "0x0000000000000000000000000000000000000000", // timer ], }); await hre.deployments.save("SpokePool", spokePoolDeployment);

Thoughts?

hhmm. I personally think the deployment artefacts should be named according to the contract file names i.e the json files produced during a migration should be a 1:1 mapping to the contracts in the contracts directory. this is what hardhat expects and could break other things if we modify this.

what we could do is modify the export of the deployments.json file, that is constructed here:
https://github.com/across-protocol/contracts-v2/blob/master/scripts/processHardhatExport.ts

this could be modified to export the *_SpokePool to simply be SpokePool on the respective chain, thereby mitigating the need for this logic all together within the export.

I've implemented this change, please take a look.

I don't think this will break anything. We're just adding a second record of the same contract.

So you'll have "Polygon_SpokePool" and "SpokePool" on polygon with all the same details.

Hardhat will still have the artifact it expects, but we will also export a second artifact under a more generic name.

Signed-off-by: chrismaree <christopher.maree@gmail.com>

chrismaree · 2022-05-11T09:43:15Z

deployments/deployments.json

    "Arbitrum_Adapter": { "address": "0x937958992799bF4A9a656E6596FD10d7Da5c2216", "blockNumber": 14704380 },
    "Boba_Adapter": { "address": "0x33B0Ec794c15D6Cc705818E70d4CaCe7bCfB5Af3", "blockNumber": 14716798 },
    "Ethereum_Adapter": { "address": "0x527E872a5c3f0C7c24Fe33F2593cFB890a285084", "blockNumber": 14704381 },
-    "Ethereum_SpokePool": { "address": "0x931A43528779034ac9eb77df799d133557406176", "blockNumber": 14704425 },


@mrice32 see the changes made here.

chrismaree · 2022-05-11T09:48:56Z

scripts/processHardhatExport.ts

          if (!processedOutput[chainId]) processedOutput[chainId] = {};
          const address = castExport[chainId][0]?.contracts[contractName].address;
          const blockNumber = findDeploymentBlockNumber(castExport[chainId][0].name, contractName);
+          if (/.*_SpokePool/.test(contractName)) contractName = "SpokePool"; // Strip the network name from the spoke pool in the contractName.


change to modify the export without changing the artefacts.

nicholaspai

LGTM sorry for not seeing this as I should have rebased off this into master

Signed-off-by: chrismaree <christopher.maree@gmail.com>

mrice32 · 2022-05-11T16:49:19Z

src/DeploymentUtils.ts

 }
+
+// Returns the chainId and contract name for a given contract address.
+export function getContractInfoFromAddress(contractAddress: string): { chainId: Number; contractName: string } {


Gotcha. Makes sense.

chrismaree added 2 commits May 6, 2022 16:24

nit

c758657

Signed-off-by: chrismaree <christopher.maree@gmail.com>

feat: add reverse look up for contract info from address

219fefe

Signed-off-by: chrismaree <christopher.maree@gmail.com>

chrismaree changed the title ~~chrismaree/add additional token util~~ feat: add reverse look up for contract info from address May 10, 2022

chrismaree added 2 commits May 10, 2022 11:29

Merge branch 'master' into chrismaree/add-additional-token-util

953adf7

nit

670fd92

Signed-off-by: chrismaree <christopher.maree@gmail.com>

chrismaree requested review from mrice32 and nicholaspai May 10, 2022 16:27

feat: add more logic

9fd590b

Signed-off-by: chrismaree <christopher.maree@gmail.com>

daywiss reviewed May 10, 2022

View reviewed changes

mrice32 reviewed May 10, 2022

View reviewed changes

chrismaree added 2 commits May 11, 2022 11:42

wip

1b92407

Signed-off-by: chrismaree <christopher.maree@gmail.com>

wip

a84d244

Signed-off-by: chrismaree <christopher.maree@gmail.com>

chrismaree commented May 11, 2022

View reviewed changes

chrismaree requested a review from mrice32 May 11, 2022 09:49

chrismaree mentioned this pull request May 11, 2022

feat: add back get target logic and update how we use contracts-v2 export for getContractInfoFromAddress across-protocol/relayer#41

Merged

nicholaspai approved these changes May 11, 2022

View reviewed changes

chrismaree added 2 commits May 11, 2022 17:00

wip

1c66296

Signed-off-by: chrismaree <christopher.maree@gmail.com>

wip

c84f6ac

Signed-off-by: chrismaree <christopher.maree@gmail.com>

mrice32 approved these changes May 11, 2022

View reviewed changes

chrismaree merged commit 087ceef into master May 12, 2022

chrismaree deleted the chrismaree/add-additional-token-util branch May 12, 2022 09:18

This was referenced May 17, 2022

fix: address issue with weth on boba #159

Merged

fix: add haircut proposal #160

Merged

fix: address param ordering issue #163

Merged

feat: add reverse look up for contract info from address #155

feat: add reverse look up for contract info from address #155

Uh oh!

Conversation

chrismaree commented May 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

daywiss May 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mrice32 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chrismaree May 11, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chrismaree May 11, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nicholaspai left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

chrismaree commented May 10, 2022 •

edited

Loading

daywiss May 10, 2022 •

edited

Loading

chrismaree May 11, 2022 •

edited

Loading

chrismaree May 11, 2022 •

edited

Loading