Use new network ID for forked chain

[kumavis]

Metamask uses the network id to determine what tx history to show.
I think it makes the most sense to show a new network id to indicate modeling a separate chain (that has been seeded with existing data).

this does not affect on-chain behavior.

a user could always override the network id to be the same as the original, if desired

relevant issue in metamask:
MetaMask/metamask-extension#783

[tcoulter]

It was an intentional decision to use the same network id because you're simulating the network you forked from (in many cases, if not all). As well, other than choosing a network id of new Date().getTime(), any relational id to the one forked from would be arbitrary and cause similar conflicts.

a user could always override the network id to be the same as the original, if desired

I was about to tell you that the same is true if you want the network id to be different, as you can override it easily. However, I just realized that this long-standing feature is undocumented. You can change it via the following:

$ testrpc -i 1337
$ testrpc --network-id 1337

as well as

var TestRPC = require("ethereumjs-testrpc"); 
var provider = TestRPC.provider({
  network_id: "1337"
});

I think the solution here is to document that feature.

[kumavis]

use the same network id because you're simulating the network you forked from (in many cases, if not all).

I understand your motivation here, but I think developers are only interested in simulating the state from the fork source, and not the network id. I think cloning the network id by default will cause more issues for developers that it will solve, as rpc-consuming block explorers and browsers will make assumptions (e.g. caching, tx history) based on the network id.

[benjamincburns]

@kumavis - do you see this causing harmful effects for MetaMask users or others, or is it just for the convenience of not needing to specify an overridden network ID?

I personally don't have a strong opinion one way or the other. Given that it makes using TestRPC's forking feature a little more straightforward for MetaMask users, I'd be willing to make the change you suggest, but my concern is that there's a very small chance that it'd break someones tests which are based on a forked chain and which use the network id to validate this starting condition.

[mikeseese]

@davidmurdoch, this is a pretty old issue. My personal preference is to leave things unchanged, but I want to say I've recently had issues with MetaMask with a networkId of 1 for a forked ganache. I feel like regardless of the default option, we should still fix this issue as I believe there's a strong use case for setting ganache's network ID to 1. In other words, if we changed the default, users would still want to set it to 1 and we'd still have to handle the issues

I'm preferring closing this issue in favor of some more exhaustive testing of the state of things today and creating new issues for anything we find in recent versions of tools

Thoughts?

[davidmurdoch]

@tcoulter since you are considering taking on the forking port for the next breaking change release, do you want to revisit the idea of changing the default net_version?

[davidmurdoch]

Closing this one as we've decided the current behavior is the desired behavior.