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

Add fork feature #405

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from
Draft

Add fork feature #405

wants to merge 8 commits into from

Conversation

joewagner
Copy link
Collaborator

@joewagner joewagner commented Jun 28, 2023

Overview

This is a proposed new feature of Local Tableland that automates starting the local tableland network as a fork of homestead (mainnet) at whatever block the user would like.

Details

There's basically three ways to use this feature.

  1. new LocalTableland({ fork: "<api provider url>", forkBlockNumber: <block number to fork from> });
  2. terminal-prompt$ npx local-tableland --fork <api provider url> --fork-block-number <block number to fork from>
  3. terminal-prompt$ FORK=<api provider url> FORK_BLOCK_NUMBER=<block number to fork from> npx local-tableland

All of these amount to passing in a url for a valid provider, and optionally including a block number to start the fork from. If no block number is sent, the provider should fork from the most recent block they consider valid.

Under the hood we are just using the Hardhat feature described here: https://hardhat.org/hardhat-network/docs/guides/forking-other-networks

Notes

A couple of things jump out at me:

  • this is mainnet and not a lot of tables here, but even so running the network with silent: false spits out a ton of logs, i.e. every tableland transaction ever on mainnet.
  • Since we are forking mainnet, the chainId becomes 1. This is fine, but potentially confusing if you're doing some local app development and you try to use your normal browser wallet.
  • If I'm reading the Hardhat docs correctly, there's potential to fork a different chain. This sounds a little complicated, but if it would be a big value add for people maybe we consider doing that as another PR after this one.

To see this in action:

  • clone this repo
  • checkout this branch
  • get yourself a provider url with an api key
  • do npm install
  • do FORK=https://mainnet.infura.io/v3/<your api key> FORK_BLOCK_NUMBER=16148005 npm run up. the block number is optional

@sanderpick
Copy link
Member

sanderpick commented Jun 29, 2023 via email

@joewagner
Copy link
Collaborator Author

There should be an infura and alchemy login in 1pass. Ask Aaron if you can’t find it.

@asutula I can see some keys in infura and alchemy, but I can't tell if any of them are available to github actions like this one. Do you have the permissions to see if that's setup for any of our other repos? I believe I'm able to create a new one, but if something already exists I think it would make sense to follow the same pattern.

@joewagner
Copy link
Collaborator Author

@asutula Disregard, I have permission in this repo to add secrets (i think this is the only one). I used the Infura key called "broker" since our infura account already has it's max number of keys, and that one didn't look like it was getting used much.

Copy link
Collaborator Author

@joewagner joewagner left a comment

Choose a reason for hiding this comment

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

This is RFR

expect(sessionOne.id).to.eql(1);
expect(sessionOne.owner).to.eql(
"0xdd8a5674eca6f1367bd0398d4b931d5b351c0be6"
);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These tests are checking values that existed at the specific block height set above.

"Chains": [
{
"Name": "Local Hardhat",
"ChainID": 1,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As noted elsewhere, the hardhat chainId is now 1. I haven't really explored the implications of this, but I would assume that dev's will have to reconfigure their a browser wallet to handle this.

"AllowTransactionRelay": false,
"Registry": {
"EthEndpoint": "ws://localhost:8545",
"ContractAddress": "0x012969f7e3439a9B04025b5a049EB9BAD82A8C12"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is the Registry proxy as deployed on mainnet.

"ContractAddress": "0x012969f7e3439a9B04025b5a049EB9BAD82A8C12"
},
"Signer": {
"PrivateKey": "ac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

All of the same standard Hardhat Wallets are setup with 10,000 ETH

hardhatCommandArr.push("--fork");
hardhatCommandArr.push(config.fork);
if (config.forkBlockNumber) {
hardhatCommandArr.push("--fork-block-number");
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is where the JS class config is set, but the dev can also use the command flags or env vars to specify.

src/main.ts Outdated
detached: !isWindows(),
cwd: this.registryDir,
env: {
...process.env,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

process.env might also have fork config.

logSync(
spawnSync(
isWindows() ? "npx.cmd" : "npx",
["hardhat", "run", "--network", "localhost", "scripts/deploy.ts"],
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We only deploy the registry if NOT using a fork, since the fork already has the registry deployed.

const binPath = getBinPath();
if (!binPath) {
throw new Error(
`cannot start with: arch ${process.arch}, platform ${process.platform}`
);
}

this.validatorDir = fork ? this.validatorForkDir : this.validatorCleanDir;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The Validator needs to know which config to use, based on if they are using a fork.
Worth noting that the ValidatorDev class, which is the docker based network for protocol development, cannot use a fork. I don't see that as being useful, but if anyone on the core team finds themself wanting that I can set it up.

@joewagner joewagner marked this pull request as ready for review June 29, 2023 18:34
@joewagner joewagner changed the base branch from main to staging June 29, 2023 20:44
@joewagner joewagner changed the title initial setup of fork feature Add fork feature Jun 29, 2023
Copy link
Contributor

@datadanne datadanne left a comment

Choose a reason for hiding this comment

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

Hm I don't think inheriting the chain id from the forked chain works, I tried adding a test case that uses a fork and creates a new table and reads from it and it fails with an EXEC_ERROR: "unsupported chain id" that comes from the validator (test case in a draft PR here if you want to look at it: #407)

If you run a regular local hardhat node and fork mainnet the local node still uses chain id 31337. I tried running this version of the code locally with a mainnet fork and the hardhat node that we bring up still uses chain id 31337:

$ FORK=https://eth-mainnet.g.alchemy... npm run up
...
...
$ cast chain-id --rpc-url http://localhost:8545
31337

I think it must be this way for security reasons too, you never want to sign a transaction for a chain id that is used by a live chain since that same transaction would be valid if it somehow gets submitted to a live rpc I think?

--

Playing with this made me think of some interesting UX questions too.
If I think about how I would ideally like to use this to work on the garage, what I would to do is:

  • get a snapshot of live data so that I don't have to recreate realistic test data. our data is spread across tables on multiple chains: we have pilot sessions-data in a mainnet table, rig attribute data in a table on arbitrum nova and ft rewards in a table on filecoin

  • I would want to be able to query and mutate all of that data via local-tableland, and use a local node that is forked from eth mainnet so that I can interact with a copy of the live contract and use a mainnet NFT metadata api for pilots

That sounds kind of hard though :D I could potentially use a staging environment deployment of the garage for development instead, but that would mean that I would have to submit every transaction to a real testnet and deal with testnet faucets etc., and it makes querying for NFT metadata annoying

@joewagner joewagner marked this pull request as draft July 3, 2023 16:27
@joewagner
Copy link
Collaborator Author

@datadanne
I think there's also a problem with NOT using chainId === 1. The contract calls to create tables include the chainId in the argument. If you look at this example the table name ends with _1 which is required for the Validator to process the create statement. This means the Validator is only going to operate correctly on the forked chain if it has the same chainId. There might be a way to work around this, but I would need to dig into the Validator implementation.

RE: security concerns.
We can set a custom chainId with hardhat. For the sake of discussion, I merged your branch and pushed change that uses chainId of 1. It looks like others want, and have done, the same thing. Examples: 1 2.
As for if we should do that, I'm open to opinions, but I was thinking the only wallets that would ever be used are the Hardhat Wallets which all have public secret keys. However, this doesn't let you do things on behalf of mainnet wallets, e.g. you can't fly a rig.

RE: ux questions.
For the first bullet:
This would require starting a hardhat fork chain for each of our supported chains. Assuming we are ok with the underlying modification of chainId in the most recent commit, we should in theory be able to support this. However I've never tried to start that many hardhat nodes on a single machine. Doesn't hurt to try, but I would guess there will be some friction.

For the second bullet:
Since the fork is an actual fork, in order to mutate data that exists on the public chain the testing infra would have to control the wallet that owns the actual asset (e.g. rig). This probably isn't a good idea for the security reasons discussed above. Maybe this means this feature isn't very useful?

@datadanne
Copy link
Contributor

I think there's also a problem with NOT using chainId === 1. The contract calls to create tables include the chainId in the argument. If you look at this example the table name ends with _1 which is required for the Validator to process the create statement. This means the Validator is only going to operate correctly on the forked chain if it has the same chainId. There might be a way to work around this, but I would need to dig into the Validator implementation.

Yeah for sure, without changing things it won't work at all unless we keep the chainId from the fork. But I think it would be better to not keep the chainId and add workarounds instead. Or I guess it depends on the use case, if it is only used for integration tests it doesn't really matter but the use case I had in mind was to use a long running node that is forked and use it when I interact with the garage in a browser while developing, and for that it feels weird to reuse the id.

Hardhat supports modifying the contract bytecode at an address, so we could for instance replace the contract after forking with one that replaces the chain id in all events that are emitted. That wouldn't make everything just magically work, but it feels like there should be some tricks we could use to glue everything together

RE: security concerns.
We can set a custom chainId with hardhat. For the sake of discussion, I merged your branch and pushed change that uses chainId of NomicFoundation/hardhat#2167. It looks like others want, and have done, the same thing.
As for if we should do that, I'm open to opinions, but I was thinking the only wallets that would ever be used are the Hardhat Wallets which all have public secret keys. However, this doesn't let you do things on behalf of mainnet wallets, e.g. you can't fly a rig.

Yeah, I 100% agree that no private key that is used on a prod chain and controls real assets should be used in testing/development. But I'm not sure that that will be obvious for all developers that use the tableland tooling in the future, they might not understand all the details of how pks/accounts work. So I think it would be ideal to try design this so that it is almost impossible to make a mistake like that (hardhat doesn't mention that you can re-use the chainId when you fork another network in their docs even though they do support it, which makes me think that it is a "only use this if you know what you are doing"-feature)

RE: ux questions.
For the first bullet:
This would require starting a hardhat fork chain for each of our supported chains. Assuming we are ok with the underlying modification of chainId in the most recent commit, we should in theory be able to support this. However I've never tried to start that many hardhat nodes on a single machine. Doesn't hurt to try, but I would guess there will be some friction.

Yeah.. I have a feeling that the developer experience would be too messy there. But it is very possible that our setup is an edge case and that we shouldn't worry about supporting multiple networks at once for now?

For the second bullet:
Since the fork is an actual fork, in order to mutate data that exists on the public chain the testing infra would have to control the wallet that owns the actual asset (e.g. rig). This probably isn't a good idea for the security reasons discussed above. Maybe this means this feature isn't very useful?

This is actually not an issue! All common test networks like hardhat, anvil etc. support impersonating any account so you could easily do things like delegate access from a mainnet account to one of the hardhat accounts with an initialization script, and then you could act on behalf of the wallet that owns the rig via standard rpc calls from a browser wallet without knowing the pk that owns the asset

https://hardhat.org/hardhat-network/docs/guides/forking-other-networks#impersonating-accounts
https://book.getfoundry.sh/reference/anvil/?highlight=impersonate#description

@joewagner
Copy link
Collaborator Author

@datadanne Overall sounds like we agree that this is useful but pretty complicated.
My intuition is to do our best to make this feature work for whatever use-case you find useful developing for Rigs, the Garage, ect...
I had not realized the account impersonation feature existed, seems like that will be really helpful. Also potentially setting up a modified contract sounds like a worthy path to explore.
I'm a little skeptical we will be able to make a fork work without keeping the chainId the same, but it seems like it's worth exploring. Do you want to do some experimenting and see if you can get this working?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants