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

[AVAX] Merge Cortina 18 (v1.10.18) #359

Draft
wants to merge 270 commits into
base: evlekht/new-dev-c19
Choose a base branch
from

Conversation

evlekht
Copy link
Member

@evlekht evlekht commented Jul 24, 2024

Why this should be merged

This PR contains changes from ava-labs/avalanchego between cortina-17 and cortina-18.
https://github.com/ava-labs/avalanchego/releases/tag/v1.10.18

How this works

Conflicts

Github workflows

We had different workflows in comparison to avax prior to this. And some common files were removed by avax. Conflicts were resolving by keeping our change and further addressed in next commits if needed.

Text (.md) files

Keeping our change. Stuff like system requirements must be revised later.

  • CONTRIBUTING.md
  • RELEASES.md
  • SECURITY.md

License header changes

Avax updated all code files license headers changing year from 2023 to 2024, since 2024 has come :). This conflicted with all files were we updated license header with c4t. All . go files listed in c666bc0 has such conflicts. A lot of files also have other conflicts.

app/app.go

Logo & code blocks structure

Codec

A lot of changes by avax. Tags removed from struct fielder, while we have upgradeVersion tag in it. Initially I kept avax changes to figure them out later, see post-merge fixes.

  • codec/reflectcodec/struct_fielder.go
  • codec/reflectcodec/type_codec.go

Genesis

Initially decided to keep fuji and mainnet files in order to avoid future conflicts, but than decided against it, see post-merge commit.

  • genesis/genesis_fuji.go
  • genesis/genesis_mainnet.go

network/network.go

authenticateIPs and peerIPStatus was modified by us (nodeID) and then removed by avax.

node/node.go

initVMs platformconfig.Config phase timestamp fields

version/constants.go

Usual versions conflict.

vms/platformvm/service.go

Avax refactored tx-builders handlers code, splitting them into 2 funcs. Handlers were modified by us with getKeystoreKeys function.

vms/platformvm/vm.go

vm.Initialize changes in codec and network creation.

PlatformVM codecs

  • vms/platformvm/block/codec.go
  • vms/platformvm/txs/codec.go
    init() logic moved into its own func, init had our changes.

vms/platformvm/block/builder/builder.go

Avax changed last lines in buildBlock func, while we added some lines before that.

vms/platformvm/block/builder/builder_test.go

Avax removed TestBuildBlock. Its test with mock, we added deferred stakers related lines to it.

vms/platformvm/block/executor/verifier.go

Avax splitted standardBlock func into two, it has our CaminoStandardTxExecutor wrapping StandardTxExecutor.

vms/platformvm/utxo/mock_verifier.go

Mocks were regenerated and vars now has real names. We added additional arg to VerifySpendUTXOs.

vms/proposervm

  • Test snow context creation was changed and its always next to nodeID generation, which was changed by us.
  • block.Build calls was moved around in a few places and this method was modified by us.
  • proVM.stakingCertLeaf -> proVM.StakingCertLeaf, and this always passed as arg into func next to arg added by us.

Affected files:

  • vms/proposervm/batched_vm_test.go
  • vms/proposervm/block.go
  • vms/proposervm/post_fork_block_test.go
  • vms/proposervm/pre_fork_block_test.go
  • vms/proposervm/state_syncable_vm_test.go
  • vms/proposervm/vm_test.go

vms/proposervm/block/block.go

Renamed var conflict and they also changed lines next to nodeID generation.

wallet/chain

Imports conflict.

  • wallet/chain/p/context.go
  • wallet/chain/x/context.go

mocks

scripts/mocks.mockgen.txt added line, while we also added some lines.
scripts/mock.gen.sh we didn't adopt previous license headers code in mocks, since it was conflicting with other license stuff. And now avax removed license headers from mocks and also added support for previously failing mocks by using source mode in mockgen.

Post-merge fixes

AVM service TestGetAssetDescription

vms/avm/camino_service_test.go TestGetAssetDescriptionC4T is removed because its mostly a duplicate and its check moved into avax vms/avm/service_test.go TestGetAssetDescription.

VMs components message codec

vms/components/message/camino_codec.go was deleted because avax added their own codec to package vms/components/message/codec.go. CaminoRewardMessage type registration moved into avax codec as first registered type. Added readme file to explain codec.

PVM tx executor TestCaminoRewardValidatorTx

vms/platformvm/txs/executor/camino_tx_executor_test.go TestCaminoRewardValidatorTx expected after-reward utxo output index changed from 1 to 4, because test genesis is changed and it affected validators ordering.

Validators start-time

vms/platformvm/txs/executor/camino_tx_executor.go starting with Durango phase, validators added with avax AddValidatorTx won't use start-time defined in tx and will use on-execution chain-time instead, same was done for CaminoAddValidatorTx.

As result of avax changes, state.NewCurrentStaker now also requires start-time arg, affecting following files:

  • vms/platformvm/camino_vm_test.go
  • vms/platformvm/state/camino_stakers.go
  • vms/platformvm/state/camino.go
  • vms/platformvm/txs/executor/camino_tx_executor_test.go

NodeID & Staking certificate

Avax encapsulated ClaimedIPPort struct creation logic in utils/ips/claimed_ip_port.go NewClaimedIPPort and that lead to cascade of changes.

At some point avax introduced staking.Certificate structure, that contains raw tls cert bytes and no parsed certificate.

NewClaimedIPPort accepts staking.Certificate as arg. And since we're generating node id not from raw cert bytes, but from parsed cert extension, we can't use raw bytes. In some places parsing happening right before NewClaimedIPPort call, but in different scope, and in some places there is no parsing at all. Also, parsing can error, while NewClaimedIPPort can't.

The least invasive and least complex way to adapt seemed to add nodeID to staking.Certificate and that lead to changes in staking.CertificateFromX509 func, which now also does nodeID generation and can return error. Now all nodeID generation calls from us are replaced with accessing cert nodeID field.

That also lead to changes in code which calls staking.CertificateFromX509, since it now returns error. Some callers of staking.CertificateFromX509 were already returning errors, but some were not, but even those changes looks safe and small.

And that lead to removal of vms/proposervm/block/build.go``Build func nodeID arg that was added by us earlier.

Affected files:

  • chains/manager.go
  • network/certs_test.go
  • network/network_test.go
  • network/peer/peer_test.go
  • network/peer/upgrader.go (simplified)
  • node/node.go
  • staking/certificate.go
  • staking/parse.go
  • tests/fixture/tmpnet/node.go
  • tools/cert/main.go (cause of logic moved to different pkg & renaming)
  • vms/proposervm/batched_vm_test.go
  • vms/proposervm/block.go (simplified)
  • vms/proposervm/post_fork_block_test.go (simplified)
  • vms/proposervm/pre_fork_block_test.go (simplified)
  • vms/proposervm/state_syncable_vm_test.go
  • vms/proposervm/vm_test.go (simplified)
  • vms/proposervm/block/block.go (simplified)
  • vms/proposervm/block/build.go (simplified)
  • vms/proposervm/block/build_test.go
  • vms/proposervm/block/parse_test.go
  • vms/proposervm/state/block_state_test.go (simplified)

TestParseDuplicateExtension

Parse func logic was changed in cortina 18. Now it can allow certificates with duplicate extensions if Durango phase is active. Its checked in vms/proposervm/block/parse_test.go TestParseDuplicateExtension, which uses some hardcoded block bytes hex. And this block certificate doesn't have camino extension which is required for nodeID generation, so we had to create new block and use its hex instead.

vms/proposervm/block/camino_test.go handles block & certificate generation.

ParseCertificatePermissive

staking/parse.go ParseCertificatePermissive is used by proposerVM block initialization if Durango phase is active. Basically, its semi-manual certificate parsing without storing parsed results and with exclusion of some normal parsing checks. Post-merge fixes updates this func to also parse extensions, generate nodeID out of them and store it. Extended certificate parsing logic (getting & checking extensions) resides in staking/camino.go. utils/crypto/secp256k1/camino_secp256k1.go nodeID generation out of extension and pub key was incapsulated in RecoverSecp256PublicKeyFromExtension, so RecoverSecp256PublicKey is using it.

Its indirectly test in vms/proposervm/block/parse_test.go and network/certs_test.go and directly tested in staking/parse_test.go. Latter requires staking/large_rsa_key.cert (oversized cert), so staking/camino_test.go implements certificate generation with correct extensions.

Since network/certs_test.go is also using hardcoded key & cert files, this PR also updates those files, generation logic is in network/camino_test.go.

Not active proposals and dropped txs

Cortina 18 changes pvm mempool gossiping logic: pvm network now has its own gossiping mempool. gossipMempool.Add(tx) reject txs that were previously dropped, which basically stops node from accepting addVoteTxs for proposal, if someone tried to vote for this proposal before it started.
Post-merge fixes changes reject condition to ignore drop-reason equal to not-yet-active dac proposal. It also refactors dac proposals IsActiveAt(time time.Time) bool -> VerifyActive(time time.Time) error by splitting ErrProposalInactive into two new errors: ErrNotActive and ErrNotYetActive.

Camino-Network-Runner

Since CNR is no longer used by e2e tests, this PR removes CNR submodule and workflows, cause some of CNR e2e tests were failing - doesn't look right to update/fix CNR in this caminogo cortina-merge PR.

Mocks regeneration

Avax added previously left-out mocks to auto-generation by using mockgen source mode. There is dedicated file that lists such mocks.
Our fixes add to this file our own mocks that were excluded from normal generation for similar reasons.

scripts/mock.gen.sh is greatly updated by this PR, it now also checks all mock files that they are listed either in mocks.mockgen.txt, or in mocks.mockgen.source.txt or in script itself as exclusions from generation.
Avax also merged 4 pvm mocks into 1 file: Versions, State, Diff and Chain. Since its still not possible to generate 4 mocks from 4 files into 1 file, our fixes are adding pvm vms/platformvm/state/mock_state.go as exclusion. And mocks themselves were regenerated one by one and then added to this file.
All of our other mocks were successfully regenerated with script with expected changes.
Also, there are no more licenses in mocks now.

Github workflows

Avax merged all ci workflows into one file: ci.yml.
Our fixes are removing all of our workflows files that are reflected in ci.yml and updating ci.yml jobs accordingly.

PVM network RewardsImportTx building lock

Node uses more granular mutexes now instead of a few common ones. network.issueTx doesn't have its own mutex and lock is happening twice further inside gossipMempool.Add call. First with txExecutorBackend mutex (its passed down with its mutex via txVerifier) and then with mempool's own mutex.

caminoNetwork CrossChainAppRequest calls txBuilder.NewRewardsImportTx. And tx builder doesn't have its own lock, so it must be handled by its caller. Because tx builder uses the same state as txExecutorBackend do, it expceted that they'll use the same mutex. So, txExecutorBackend.lock is passed down to caminoNetwork and caminoNetwork call of txBuilder.NewRewardsImportTx is wrapped into its own method with lock-defer-unlock with caminoNetwork's mutex.

Codec

As written in conflicts section - a lot of changes by avax. Tags removed from struct fielder, while we have upgradeVersion tag in it. On conflict, avax changes were kept, just to figure them out later - in those fixes. Post-merge fixes are doing it. Avax changes in struct fielder structure are mostly incompatible, so after a few iterations I decided to keep our changes regarding GetSerializedFields method. Some code were moved into its own file.

Affected files:

  • codec/reflectcodec/struct_fielder.go
  • codec/reflectcodec/type_codec.go
  • codec/reflectcodec/camino.go
  • vms/platformvm/dac/camino_codec.go

Fuji and mainnet genesis

Initially, on conflicts resolution, I decided to keep them to reduce diff. But during post-merge fixes I reverted that decision, so we'll get "notified" if their genesis files were changed for some reason.

PVM tests helper file changes

A lot of changes, mostly cause of changes to how fork-time are set and chosen, to configs and to context creation. Our changes are rather messy and minimalistic, since I didn't want to do full scale refactoring in this merge PR. Dedicated issue was created for this.

How this was tested

With already existing unit-tests and e2e tests.

Additional references

Original PR
#318

dhrubabasu and others added 30 commits November 8, 2023 22:28
Signed-off-by: Joshua Kim <[email protected]>
Co-authored-by: Dan Laine <[email protected]>
Signed-off-by: David Boehm <[email protected]>
Co-authored-by: Stephen Buttolph <[email protected]>
Co-authored-by: Dan Laine <[email protected]>
Signed-off-by: Joshua Kim <[email protected]>
Co-authored-by: Stephen Buttolph <[email protected]>
Signed-off-by: Joshua Kim <[email protected]>
Co-authored-by: Stephen Buttolph <[email protected]>
Co-authored-by: Dan Laine <[email protected]>
Co-authored-by: Stephen Buttolph <[email protected]>
Signed-off-by: Stephen Buttolph <[email protected]>
Co-authored-by: Stephen Buttolph <[email protected]>
hugo-syn and others added 26 commits January 12, 2024 12:56
Signed-off-by: Stephen Buttolph <[email protected]>
Co-authored-by: Filippo Valsorda <[email protected]>
# Conflicts:
#	RELEASES.md
#	api/admin/client.go
#	api/admin/service.go
#	go.mod
#	go.sum
#	node/node.go
#	version/compatibility.json
#	version/constants.go
#	vms/platformvm/api/static_service.go
#	vms/platformvm/service.go
#	vms/platformvm/vm.go
# Conflicts:
#	.github/workflows/test.e2e.existing.yml
#	.github/workflows/test.e2e.yml
#	.github/workflows/test.upgrade.yml
#	README.md
#	RELEASES.md
#	genesis/bootstrappers.json
#	go.sum
#	node/node.go
#	scripts/build_tmpnetctl.sh
#	scripts/mocks.mockgen.txt
#	scripts/tests.e2e.persistent.sh
#	scripts/tests.e2e.sh
#	tests/e2e/README.md
#	tests/fixture/tmpnet/local/config.go
#	version/compatibility.json
#	version/constants.go
#	vms/platformvm/state/diff.go
#	vms/platformvm/state/mock_chain.go
#	vms/platformvm/state/mock_diff.go
#	vms/platformvm/state/mock_state.go
#	vms/platformvm/state/state.go
# Conflicts:
#	.github/workflows/auto-generated-checker.yml
#	.github/workflows/buf-lint.yml
#	.github/workflows/build-linux-binaries.yml
#	.github/workflows/build-macos-release.yml
#	.github/workflows/build-public-ami.yml
#	.github/workflows/build-ubuntu-amd64-release.yml
#	.github/workflows/build-ubuntu-arm64-release.yml
#	.github/workflows/build-win-release.yml
#	.github/workflows/static-analysis.yml
#	.github/workflows/test.e2e.existing.yml
#	.github/workflows/test.e2e.yml
#	.github/workflows/test.unit.yml
#	CONTRIBUTING.md
#	RELEASES.md
#	SECURITY.md
#	api/admin/client.go
#	api/admin/service.go
#	api/common_args_responses.go
#	api/info/client.go
#	api/info/service.go
#	api/server/server.go
#	app/app.go
#	codec/reflectcodec/struct_fielder.go
#	codec/reflectcodec/type_codec.go
#	config/config.go
#	config/flags.go
#	database/linkeddb/linkeddb.go
#	genesis/config.go
#	genesis/genesis.go
#	genesis/genesis_fuji.go
#	genesis/genesis_local.go
#	genesis/genesis_mainnet.go
#	genesis/genesis_test.go
#	genesis/params.go
#	genesis/unparsed_config.go
#	go.sum
#	header.yml
#	main/main.go
#	network/certs_test.go
#	network/network.go
#	network/peer/peer_test.go
#	network/peer/upgrader.go
#	node/node.go
#	scripts/mock.gen.sh
#	scripts/mocks.mockgen.txt
#	staking/tls.go
#	tests/e2e/banff/suites.go
#	tests/e2e/e2e_test.go
#	tests/e2e/p/permissionless_subnets.go
#	tests/e2e/p/workflow.go
#	tests/e2e/static-handlers/suites.go
#	tests/e2e/x/transfer/virtuous.go
#	tests/fixture/e2e/helpers.go
#	tests/fixture/tmpnet/config.go
#	tests/fixture/tmpnet/local/config.go
#	tests/upgrade/upgrade_test.go
#	utils/constants/application.go
#	utils/constants/network_ids.go
#	utils/constants/network_ids_test.go
#	utils/wrappers/packing.go
#	version/compatibility.go
#	version/constants.go
#	version/string.go
#	vms/avm/service.go
#	vms/components/avax/atomic_utxos.go
#	vms/platformvm/api/static_service.go
#	vms/platformvm/block/builder/builder.go
#	vms/platformvm/block/builder/builder_test.go
#	vms/platformvm/block/codec.go
#	vms/platformvm/block/executor/proposal_block_test.go
#	vms/platformvm/block/executor/standard_block_test.go
#	vms/platformvm/block/executor/verifier.go
#	vms/platformvm/block/executor/verifier_test.go
#	vms/platformvm/client.go
#	vms/platformvm/config/config.go
#	vms/platformvm/fx/fx.go
#	vms/platformvm/genesis/genesis.go
#	vms/platformvm/metrics/block_metrics.go
#	vms/platformvm/service.go
#	vms/platformvm/service_test.go
#	vms/platformvm/state/diff.go
#	vms/platformvm/state/state.go
#	vms/platformvm/txs/codec.go
#	vms/platformvm/txs/executor/atomic_tx_executor.go
#	vms/platformvm/txs/executor/proposal_tx_executor.go
#	vms/platformvm/txs/executor/staker_tx_verification.go
#	vms/platformvm/txs/executor/standard_tx_executor.go
#	vms/platformvm/txs/executor/standard_tx_executor_test.go
#	vms/platformvm/txs/executor/state_changes.go
#	vms/platformvm/txs/executor/tx_mempool_verifier.go
#	vms/platformvm/txs/visitor.go
#	vms/platformvm/utxo/handler.go
#	vms/platformvm/utxo/handler_test.go
#	vms/platformvm/utxo/mock_verifier.go
#	vms/platformvm/vm.go
#	vms/proposervm/batched_vm_test.go
#	vms/proposervm/block.go
#	vms/proposervm/block/block.go
#	vms/proposervm/block/build.go
#	vms/proposervm/block/build_test.go
#	vms/proposervm/block/parse_test.go
#	vms/proposervm/post_fork_block_test.go
#	vms/proposervm/pre_fork_block_test.go
#	vms/proposervm/state/block_state_test.go
#	vms/proposervm/state_syncable_vm_test.go
#	vms/proposervm/vm_test.go
#	vms/secp256k1fx/keychain_test.go
#	wallet/chain/p/context.go
#	wallet/chain/x/context.go
@evlekht evlekht force-pushed the evlekht/cortina-18 branch from 26c84dc to a5c21b3 Compare July 24, 2024 17:18
@evlekht evlekht force-pushed the evlekht/cortina-18 branch from 46c7f5a to e1562a5 Compare July 24, 2024 17:47
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.