-
Notifications
You must be signed in to change notification settings - Fork 31
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
Bump avago subnet evm #714
Conversation
|
||
# Don't export them as they're used in the context of other calls | ||
AVALANCHEGO_VERSION=${AVALANCHEGO_VERSION:-$(extract_commit "$(getDepVersion github.com/ava-labs/avalanchego)")} | ||
# Temporarily hardcode the Avalanchego version until outbound networking relaxation is available | ||
AVALANCHEGO_VERSION=v1.12.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not enabled by the update of avago but was possible since the switch to using an external binary for the signature aggregation in tests.
// Apply node config flags to explicitly set ports to their existing values | ||
// Otherwise nodes might start up with dynamic ports making existing clients and bound contracts | ||
// unusable | ||
// TODO: remove once tmpnet supports static port option across restarts | ||
for _, tmpnetNode := range n.Network.Nodes { | ||
port := getTmpnetNodePort(tmpnetNode) | ||
tmpnetNode.Flags[config.HTTPPortKey] = port | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we also have to do this ahead of the network restart in ConvertSubnet
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah we do the other diff on line 270 is inside ConvertSubnet
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
D'oh not sure how I missed that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense to me. Just not sure if tmpnet
is planning on supporting static ports across restarts at all going foward.
@@ -537,3 +550,12 @@ func (n *LocalNetwork) GetTwoL1s() ( | |||
Expect(len(l1s)).Should(BeNumerically(">=", 2)) | |||
return l1s[0], l1s[1] | |||
} | |||
|
|||
// TODO: once tmpnet supports static port option across restarts, remove this function |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this something that is planned to be added on tmpnet? I forget the context on that side, but I thought this support was actually removed since it's no longer needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah -- it's already added back but requiring explicit configuration: ava-labs/avalanchego#3697 not including it in this PR since would need to be on branches again and this PR allows us to use properly tagged releases.
Why this should be merged
Updates avago and subnet-evm to latest versions.
How this works
Newest changes in avago made tmpnet node ports dynamic on restart. The workaround here explicitly fixes them when restarting. This should be removed when releases are cut to re-enable static local tmpnet node ports.
How this was tested
E2E
How is this documented