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

Tests use permissionless l1 #641

Open
wants to merge 24 commits into
base: main
Choose a base branch
from
Open

Conversation

cam-schultz
Copy link
Contributor

@cam-schultz cam-schultz commented Nov 9, 2024

Why this should be merged

Adds support for Etna SoVs in LocalNetwork. Allows for full separation of primary network and SoV validator sets. Also includes many semi-related cleanup items.

How this works

The meat of this change involves moving the subnet conversion steps previously done in the validator manager test flows to LocalNetwork.

  • LocalNetwork.NewNetwork specifies only a single bootstrap node per subnet.
  • LocalNetwork.ConvertSubnet converts a subnet by:
    1. Deploying a ValidatorManager instance (the concrete instance is specfied as an argument to the call)
    2. Issuing the ConvertSubnetTx on the P-Chain
    3. Initializing the subnet validators with the specified weights
    4. Removing the bootstrap nodes as subnet validators

After the subnet is converted, the bootstrap nodes validate only the primary network, and the nodes provided in the subnet conversion only validate the subnet.

LocalNetwork users can choose if and when to convert the subnet. For example, the teleporter suite does so in the initial network setup, converting all subnets to be managed by PoAValidatorManager. The governance suite uses pre-Etna subnet validators as before. The validator-manager leaves it to each flow to convert the subnet according to its needs.

Additionally, cleans up the following:

  • Adds a getter for the ERC20TokenStakingManager's staking asset
  • Uses signature-aggregator over the Subnet EVM Warp API to aggregate signatures across all tests
    • There seemed to be some timing inconsistencies with the Warp API that were not present when fetching the signatures directly.
  • Generalizes many functions in validator_manager.go to use IValidatorManager or IPoSValidatorManager instances to remove duplicate code.

How this was tested

CI

How is this documented

N/A

@cam-schultz cam-schultz changed the base branch from main to simplify-local-network-struct November 9, 2024 00:22
/**
* @notice Returns the ERC20 token being staked
*/
function erc20() external view returns (IERC20Mintable) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added this to allow the deployment utility to return an IPoSValidatorManager object, and recover the ERC20 instance after the fact. It also seems reasonable that use cases will emerge that will want to access the staked asset programatically.

--exc $filtered_contracts
echo "Generating Go bindings for $contract_name..."

if [ -z "$filtered_contracts" ]; then
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This addresses an edge case in which the only contract in the combined JSON is the contract to compile itself. In that case, there are no contracts to exclude.

@@ -202,6 +363,37 @@ func (n *LocalNetwork) GetPrimaryNetworkInfo() interfaces.SubnetTestInfo {
}
}

func (n *LocalNetwork) GetSubnetInfo(subnetID ids.ID) interfaces.SubnetTestInfo {
for _, subnet := range n.Network.Subnets {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are a handful of places where we do a linear search over the subnets and nodes. I didn't think it was worth migrating to maps for random access, given that we're only ever searching over a handful of subnets/nodes.

Expect(err).Should(BeNil())

return proxyAddress, proxyAdmin, contract
return proxyAddress, proxyAdmin
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The target T contract instance can be constructed from the proxyAddress by the caller.

@cam-schultz cam-schultz marked this pull request as ready for review November 12, 2024 21:16
@cam-schultz cam-schultz requested a review from a team as a code owner November 12, 2024 21:16
return n.pChainWallet
// Create the P-Chain wallet to issue transactions
kc := secp256k1fx.NewKeychain(n.globalFundedKey)
n.GetSubnetsInfo()
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this line is necessary

@@ -57,7 +58,7 @@ var _ = ginkgo.BeforeSuite(func() {
Expect(err).Should(BeNil())

// Create the local network instance
ctx, cancel := context.WithTimeout(context.Background(), 120*time.Second)
ctx, cancel := context.WithTimeout(context.Background(), 120*2*time.Second)
Copy link
Contributor

Choose a reason for hiding this comment

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

Would this make more sense as 60*4*time.Second?

Base automatically changed from simplify-local-network-struct to main November 13, 2024 21:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Backlog 🗄️
Development

Successfully merging this pull request may close these issues.

2 participants