Skip to content

Conversation

0xIcarus
Copy link

Description

PR deprecates usage of governed gas pool and sets up stake.move with the right prerequisite function calls to enable voting COLLECT_AND_DISTRIBUTE_GAS_FEES feature flag through using the release testing method

Type of Change

  • New feature
  • Bug fix
  • Breaking change
  • Performance improvement
  • Refactoring
  • Dependency update
  • Documentation update
  • Tests

Which Components or Systems Does This Change Impact?

  • Validator Node
  • Full Node (API, Indexer, etc.)
  • Move/Aptos Virtual Machine
  • Aptos Framework
  • Aptos CLI/SDK
  • Developer Infrastructure
  • Other (specify)

How Has This Been Tested?

Key Areas to Review

Checklist

  • I have read and followed the CONTRIBUTING doc
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I identified and added all stakeholders and component owners affected by this change as reviewers
  • I tested both happy and unhappy path of the functionality
  • I have made corresponding changes to the documentation

@0xIcarus 0xIcarus marked this pull request as draft July 12, 2025 08:54
@0xIcarus 0xIcarus marked this pull request as ready for review July 30, 2025 08:00
Copy link

@sebtomba sebtomba left a comment

Choose a reason for hiding this comment

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

LGTM

@0xmovses 0xmovses self-requested a review July 31, 2025 15:21
Copy link
Collaborator

@0xmovses 0xmovses left a comment

Choose a reason for hiding this comment

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

This looks good. I have one move unit failing test movement move test. I know there were many more failing and you've solved a lot of them. Almost there.

Failures in 0x1::aptos_governance:

┌── test_stake_pool_can_vote_before_and_after_partial_governance_voting_enabled ──────
│ error[E11001]: test failure
│     ┌─ /Users/movses/mvmt/mvmt-aptos-core/aptos-move/framework/aptos-framework/sources/aptos_governance.move:752:13
│     │
│ 750 │     fun assert_voting_initialization() {
│     │         ---------------------------- In this function in 0x1::aptos_governance
│ 751 │         if (features::partial_governance_voting_enabled()) {
│ 752 │             assert!(exists<VotingRecordsV2>(@aptos_framework), error::invalid_state(EPARTIAL_VOTING_NOT_INITIALIZED));
│     │             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Test was not expected to error, but it aborted with code 196621 originating in the module 0000000000000000000000000000000000000000000000000000000000000001::aptos_governance rooted here
│
│
│ stack trace
│ 	aptos_governance::get_remaining_voting_power(/Users/movses/mvmt/mvmt-aptos-core/aptos-move/framework/aptos-framework/sources/aptos_governance.move:312)
│ 	aptos_governance::test_stake_pool_can_vote_before_and_after_partial_governance_voting_enabled(/Users/movses/mvmt/mvmt-aptos-core/aptos-move/framework/aptos-framework/sources/aptos_governance.move:1162)
│
└──────────────────

Test result: FAILED. Total tests: 499; passed: 498; failed: 1
{
  "Error": "Move unit tests failed"
}

Furthermore, we should deprecate the GGP feature using the upstream procedure. See for example this _DEPRECATED... feature from the upstream repo: https://github.com/aptos-labs/aptos-core/blob/main/aptos-move/aptos-release-builder/src/components/feature_flags.rs#L235

Instead of removing the feature from the enum, follow the example. Hopefully there are not too many gotyas.

Also make sure to run all the rust e2e tests that utilise the MoveHarness cargo test

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.

3 participants