movementlabsxyz / aptos-core

Aptos is a layer 1 blockchain built to support the widespread use of blockchain through better technology and user experience.
https://aptoslabs.com
Other
13 stars 18 forks source link

[Bug] No change of epochs will block governance #82

Open andygolay opened 2 weeks ago

andygolay commented 2 weeks ago

πŸ› Bug

Because Suzuka epoch is fixed at 1, functions that rely on aptos_governance::reconfigure won't work.

For example this script to enable feature flags:

script {
    use aptos_framework::aptos_governance;
    use std::features;

    fun enable_bridge_feature(core_resources: &signer) {
        let core_signer = aptos_governance::get_signer_testnet_only(core_resources, @0x1);

        let framework_signer = &core_signer;

        let enabled_blob: vector<u64> = vector[
            features::get_atomic_bridge_feature()
        ];

        let disabled_blob: vector<u64> = vector[];

        features::change_feature_flags_for_next_epoch(framework_signer, enabled_blob, disabled_blob);
        aptos_governance::reconfigure(framework_signer);
    }
}

To reproduce

Run the above script. You'll see the feature flag in features::PendingFeatures.

Expected Behavior

We need some way to do governance without the epoch change. Perhaps reconfiguration::reconfigure could advance to the next block instead, so that governance proposals are enacted on next block, rather than next epoch? This would require input from @mzabaluev / @l-monninger others who are more familiar with the inner workings for the repository, as far as what side effects or subtle issues that change might entail.

System information

Additional context

This is blocking client integration tests with the atomic bridge framework modules, because those modules are feature flagged out.

andyjsbell commented 2 weeks ago

Maybe you could use features::on_new_epoch(&framework_signer); instead of aptos_governance::reconfigure(framework_signer);

https://github.com/movementlabsxyz/aptos-core/blob/70be3926ff79ff4cdb0cee928f717fafcd41ecdd/aptos-move/e2e-move-tests/src/tests/bridge.data/atomic_bridge_feature/sources/main.move#L13

0xmovses commented 2 weeks ago

Catching up here, wouldn't on_new_epoch still not work as there is only ever one epoch on movement testnet. The epoch would never change.

andygolay commented 2 weeks ago

Looks like it worked πŸ™

on_new_epoch sets the flag as if there's a new epoch.

Do you think this issue should stay open or is it already known @andyjsbell ?

image
andyjsbell commented 2 weeks ago

Good. This will depend if this is by design so maybe @l-monninger or @mzabaluev can chip in on this?