Repository navigation
fix(evm-nwo): stop cross-network chain reuse, add graphHiding and image overrides - #2418
Conversation
Effi-S
left a comment
Effi-S
left a comment
There was a problem hiding this comment.
Nit: network-key format duplicated across two call sites
integration/nwo/token/evm/nwo.go:132 and :245
tms.Network + ":" + tms.Channel is written in both GetEntry and startNode. If the format changes in one place only, reuse silently breaks; a keyOf(tms) helper would keep them in lockstep.
Nit: GraphHiding knob not settable from any topology
integration/nwo/token/evm/nwo.go:83
p.GraphHiding (like p.Image, p.ChainID, p.Threshold) is never wired from the factory/topology, so it's only enablable by direct field assignment. Intentional per PR description; noted for completeness.
ac309c7 to
50abe65
Compare
50abe65 to
370c0c3
Compare
|
Pushed 370c0c3 (rebased on main). Added a networkKey(tms) helper so GetEntry and startNode derive the network key the same way. Left GraphHiding as a field knob as noted: nothing sets it non-default yet, so I'd rather wire it from the topology when a suite actually needs it. |
370c0c3 to
07a5ae6
Compare
|
hi @Effi-S all the reviews are addressed |
Effi-S
left a comment
Effi-S
left a comment
There was a problem hiding this comment.
Some Low severity findings
07a5ae6 to
d563026
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Effi-S
left a comment
Effi-S
left a comment
There was a problem hiding this comment.
LGTM
@AkramBitar - Any thoughts?
d563026 to
36da93e
Compare
…our image overrides startNode returned the first entry with a running node regardless of which network it belonged to, so a suite standing up two independent EVM topologies would silently settle the second one onto the first one's chain. Adds an Entry.Network field and a nodeForNetwork helper that checks it before reusing a node. The key is length-prefixed so a colon in the network or channel cannot make two networks collide. DeploySpec.GraphHiding was never populated, so every TMS deployed with graphHiding false regardless of what the token driver needed. It is now derived per TMS from the TMS's driver, so networks served by one handler can differ and there is no knob to keep in sync. An unknown driver fails loudly instead of deploying the wrong mode. And makes BESU_IMAGE / FABRICX_EVM_IMAGE actually reach the suites. Before this, overriding them only changed which image the make targets pulled; the suite itself always booted the hardcoded default image name, and CheckImagesExist failed because that one was never pulled. The handler now resolves the image (environment, then configured, then default) in one shared helper; StartBesu and startGatewayNode take the image as given and only apply the default. Signed-off-by: atharrva01 <atharvaborade568@gmail.com>
36da93e to
ca88d63
Compare
3 participants
Part of #2412 (items 7, 9, 10).
startNode returned the first entry with a running node regardless of which network it belonged to, so a suite standing up two independent EVM topologies (NewTopologyWithName) would silently settle the second one onto the first one's chain. Adds an Entry.Network field and a nodeForNetwork helper that checks it before reusing a node.
GraphHiding is now derived per TMS from its token driver (DeploySpec.GraphHiding was never populated, so every TMS deployed with graphHiding false). Both registered drivers are non graph-hiding today; an unknown driver fails loudly.
And makes BESU_IMAGE / FABRICX_EVM_IMAGE actually reach the suites. Before this, overriding them only changed which image the make targets pulled; the suite itself always booted the hardcoded default image name, and CheckImagesExist failed because that one was never pulled. StartBesu and startGatewayNode now fall back to the environment variable before the hardcoded default.
Test plan