From 9116d2ea18c3915be7d7741921c4c9a1d75ffee9 Mon Sep 17 00:00:00 2001 From: Dhaval Purohit Date: Mon, 22 Sep 2025 18:15:03 +0530 Subject: [PATCH 01/14] WIP: adding leader ban registry module in supra framework --- .../sources/configs/config_buffer.move | 1 + .../configs/leader_ban_registry_config.move | 124 ++++++ .../sources/leader_ban_registry.move | 101 +++++ .../tests/test_leader_ban_registry.move | 2 + .../supra-stdlib/sources/decode_bcs.move | 370 ++++++++++++++++++ 5 files changed, 598 insertions(+) create mode 100644 aptos-move/framework/supra-framework/sources/configs/leader_ban_registry_config.move create mode 100644 aptos-move/framework/supra-framework/sources/leader_ban_registry.move create mode 100644 aptos-move/framework/supra-framework/tests/test_leader_ban_registry.move create mode 100644 aptos-move/framework/supra-stdlib/sources/decode_bcs.move diff --git a/aptos-move/framework/supra-framework/sources/configs/config_buffer.move b/aptos-move/framework/supra-framework/sources/configs/config_buffer.move index 9a3d384827555..a2fa31c0cc8e8 100644 --- a/aptos-move/framework/supra-framework/sources/configs/config_buffer.move +++ b/aptos-move/framework/supra-framework/sources/configs/config_buffer.move @@ -32,6 +32,7 @@ module supra_framework::config_buffer { friend supra_framework::randomness_config_seqnum; friend supra_framework::version; friend supra_framework::automation_registry; + friend supra_framework::leader_ban_registry_config; /// Config buffer operations failed with permission denied. const ESTD_SIGNER_NEEDED: u64 = 1; diff --git a/aptos-move/framework/supra-framework/sources/configs/leader_ban_registry_config.move b/aptos-move/framework/supra-framework/sources/configs/leader_ban_registry_config.move new file mode 100644 index 0000000000000..a76433772c96b --- /dev/null +++ b/aptos-move/framework/supra-framework/sources/configs/leader_ban_registry_config.move @@ -0,0 +1,124 @@ +/// Provides the config related to leader ban registry +module supra_framework::leader_ban_registry_config { + use std::error; + use std::vector; + use supra_std::decode_bcs; + use supra_framework::config_buffer; + use supra_framework::system_addresses; + + friend supra_framework::genesis; + friend supra_framework::reconfiguration_with_dkg; + + /// The provided on chain config bytes are empty or invalid + const EINVALID_CONFIG: u64 = 1; + /// The provided on chain config version should be equal or greater than existing + const EINVALID_VERSION: u64 = 2; + /// Decoding from version bytes failed + const EINVALID_VERSION_BYTES: u64 = 3; + + + struct BanRegistryParameters has drop, key, store { + config: vector, + version: u8 + } + + struct BanRegistryParametersV0 has drop { + initial_elections_denied: u8, + max_elections_denied: u32, + minimum_unbanned_proposers: u8 + } + + /// Publishes the BanRegistryParameters config. + public(friend) fun initialize(supra_framework: &signer, config: vector) { + system_addresses::assert_supra_framework(supra_framework); + assert!(vector::length(&config) != 0, error::invalid_argument(EINVALID_CONFIG)); + // we always init with version 0 + move_to(supra_framework, BanRegistryParameters { config, version: 0 }); + } + + /// This can be called by on-chain governance to update on-chain configs for the next epoch. + /// Example usage: + /// ``` + /// supra_framework::leader_ban_registry_config::set_for_next_epoch(&framework_signer, some_config_bytes, version); + /// supra_framework::supra_governance::reconfigure(&framework_signer); + /// ``` + public fun set_for_next_epoch(account: &signer, config: vector, version: u8) acquires BanRegistryParameters { + system_addresses::assert_supra_framework(account); + assert!(vector::length(&config) != 0, error::invalid_argument(EINVALID_CONFIG)); + if (exists(@supra_framework)) { + let ban_registry_params = borrow_global(@supra_framework); + assert!(version >= ban_registry_params.version, error::invalid_argument(EINVALID_VERSION)); + }; + std::config_buffer::upsert(BanRegistryParameters {config, version}); + } + + /// Only used in reconfigurations to apply the pending `BanRegistryParameters`, if there is any. + public(friend) fun on_new_epoch(framework: &signer) acquires BanRegistryParameters { + system_addresses::assert_supra_framework(framework); + if (config_buffer::does_exist()) { + let new_config = config_buffer::extract(); + if (exists(@supra_framework)) { + *borrow_global_mut(@supra_framework) = new_config; + } else { + move_to(framework, new_config); + }; + } + } + + /// Provide initial election denied value efficiently + /// by decdoing correct version and if not set then 0 + public fun get_initial_election_denied(): u8 acquires BanRegistryParameters { + if (!exists::(@supra_framework)) { + let ban_registry_config = borrow_global(@supra_framework); + if (ban_registry_config.version == 0) { + let ban_registry_params = deserialise_v1_param(ban_registry_config.config); + return ban_registry_params.initial_elections_denied; + } + }; + 0 + } + + /// Provide max electoion denied value efficiently + /// by decdoing correct version and if not set then 0 + public fun get_max_elections_denied(): u32 acquires BanRegistryParameters { + if (!exists::(@supra_framework)) { + let ban_registry_config = borrow_global(@supra_framework); + if (ban_registry_config.version == 0) { + let ban_registry_params = deserialise_v1_param(ban_registry_config.config); + return ban_registry_params.max_elections_denied; + } + }; + 0 + } + + /// Provide max electoion denied value efficiently + /// by decdoing correct version and if not set then 0 + public fun get_minimum_unbanned_proposers(): u8 acquires BanRegistryParameters { + if (!exists::(@supra_framework)) { + let ban_registry_config = borrow_global(@supra_framework); + if (ban_registry_config.version == 0) { + let ban_registry_params = deserialise_v1_param(ban_registry_config.config); + return ban_registry_params.minimum_unbanned_proposers; + } + }; + 0 + } + + /// Decoding bytes to `BanRegistryParametersV0` using bcs + fun deserialise_v1_param(bytes: vector): BanRegistryParametersV0 { + let bcs_bytes = decode_bcs::new(bytes); + let initial_elections_denied: u8 = decode_bcs::peel_u8(&mut bcs_bytes); + let max_elections_denied: u32 = decode_bcs::peel_u32(&mut bcs_bytes); + let minimum_unbanned_proposers: u8 = decode_bcs::peel_u8(&mut bcs_bytes); + // making sure not bytes left to decode means correct parameter version + assert!( + vector::length(&decode_bcs::into_remainder_bytes(bcs_bytes)) == 0, + error::out_of_range(EINVALID_VERSION_BYTES) + ); + BanRegistryParametersV0 { + initial_elections_denied, + max_elections_denied, + minimum_unbanned_proposers + } + } +} diff --git a/aptos-move/framework/supra-framework/sources/leader_ban_registry.move b/aptos-move/framework/supra-framework/sources/leader_ban_registry.move new file mode 100644 index 0000000000000..db517796ca3e4 --- /dev/null +++ b/aptos-move/framework/supra-framework/sources/leader_ban_registry.move @@ -0,0 +1,101 @@ +/// Maintains the list of banned validators and updates counters on every epoch +module supra_framework::leader_ban_registry { + use std::error; + use supra_std::enumerable_map; + use supra_framework::system_addresses; + + /// Leader ban registry already initialized + const EBAN_REGISTRY_ALREADY_EXISTS: u64 = 1; + /// Leader ban registry not initialized + const EBAN_REGISTRY_NOT_INITIALIZED: u64 = 2; + /// Validator already exists in registry + const EVALIDATOR_ALREADY_EXISTS: u64 = 3; + /// Validator not exists in registry + const EVALIDATOR_DOESNT_EXISTS: u64 = 4; + + /// Holds metrics for banned round, epoch and round server in prev epoch + struct ActiveBan has store, drop { + epoch_earned: u64, // EPOCH + round_earned: u64, // ROUND + rounds_served_in_previous_epochs: u64 // ROUND + } + + /// Holds validator metrics regarding duration pool address etc + struct ValidatorBansWithAddress has store, drop { + active: ActiveBan, + consecutive_bans: u32, + pool_address: address, + } + + /// Holds ban registry + struct BanRegistry has drop, store, key { + bans: enumerable_map::EnumerableMap + } + + /// Initialise leader ban registry + public(friend) fun initialize_leader_ban_registry(supra_framework: &signer) { + system_addresses::assert_supra_framework(supra_framework); + assert!( + !exists(@supra_framework), + error::already_exists(EBAN_REGISTRY_ALREADY_EXISTS) + ); + move_to(supra_framework, BanRegistry { bans: enumerable_map::new_map() }); + } + + /// Adding an entry for new validator + public entry fun add_new_ban_entry( + supra_framework: &signer, validator_address: address, pool_address: address, epoch_earned: u64, round_earned: u64 + ) + acquires BanRegistry + { + // TODO: epoch can be retrived from reconfigure + // TODO: what about round + // TODO: Validator address and index can be fetched pool address see stake.move + + system_addresses::assert_supra_framework(supra_framework); + assert_registry_initialized(); + let ban_registry = borrow_global_mut(@supra_framework); + + if (enumerable_map::contains(&ban_registry.bans, validator_address)) { + abort error::already_exists(EVALIDATOR_ALREADY_EXISTS) + }; + + enumerable_map::add_value(&mut ban_registry.bans, validator_address, ValidatorBansWithAddress { + active: ActiveBan { + epoch_earned, + round_earned, + rounds_served_in_previous_epochs: 0 + }, + consecutive_bans:0, + pool_address + }) + } + + /// Remvoing an entry of validator from registry + public entry fun remove_ban_entry(supra_framework: &signer, validator_address: address) + acquires BanRegistry + { + system_addresses::assert_supra_framework(supra_framework); + assert_registry_initialized(); + let ban_registry = borrow_global_mut(@supra_framework); + + if (!enumerable_map::contains(&ban_registry.bans, validator_address)) { + abort error::not_found(EVALIDATOR_DOESNT_EXISTS) + }; + + enumerable_map::remove_value(&mut ban_registry.bans, validator_address) + } + + /// Update countes on every epoch + public(friend) fun on_new_epoch() { + /// TODO: What are the use cases configuration here? + } + + /// Validates registry initialised if not aborted with `EBAN_REGISTRY_NOT_INITIALIZED` + fun assert_registry_initialized() { + assert!( + exists(@supra_framework), + error::invalid_state(EBAN_REGISTRY_NOT_INITIALIZED) + ); + } +} diff --git a/aptos-move/framework/supra-framework/tests/test_leader_ban_registry.move b/aptos-move/framework/supra-framework/tests/test_leader_ban_registry.move new file mode 100644 index 0000000000000..b9b4853d1e5a7 --- /dev/null +++ b/aptos-move/framework/supra-framework/tests/test_leader_ban_registry.move @@ -0,0 +1,2 @@ +#[test_only] +module std::test_leader_ban_registry {} diff --git a/aptos-move/framework/supra-stdlib/sources/decode_bcs.move b/aptos-move/framework/supra-stdlib/sources/decode_bcs.move new file mode 100644 index 0000000000000..570a38780a42c --- /dev/null +++ b/aptos-move/framework/supra-stdlib/sources/decode_bcs.move @@ -0,0 +1,370 @@ +/// This module implements BCS (de)serialization in Move. +/// Full specification can be found here: https://github.com/diem/bcs +module supra_std::decode_bcs { + use std::vector as v; + use std::bcs; + + /// For when bytes length is less than required for deserialization. + const EOutOfRange: u64 = 0; + + /// For when the boolean value different than `0` or `1`. + const ENotBool: u64 = 1; + + /// For when ULEB byte is out of range (or not found). + const ELenOutOfRange: u64 = 2; + + /// A helper struct that saves resources on operations. For better + /// vector performance, it stores reversed bytes of the BCS and + /// enables use of `vector::pop_back`. + struct BCS has store, copy, drop { + bytes: vector + } + + /// Get BCS serialized bytes for any value. + /// Re-exports stdlib `bcs::to_bytes`. + public fun to_bytes(value: &T): vector { + bcs::to_bytes(value) + } + + /// Creates a new instance of BCS wrapper that holds inversed + /// bytes for better performance. + public fun new(bytes: vector): BCS { + v::reverse(&mut bytes); + BCS { bytes } + } + + /// Unpack the `BCS` struct returning the leftover bytes. + /// Useful for passing the data further after partial deserialization. + public fun into_remainder_bytes(bcs: BCS): vector { + let BCS { bytes } = bcs; + v::reverse(&mut bytes); + bytes + } + + /// Read a `bool` value from bcs-serialized bytes. + public fun peel_bool(bcs: &mut BCS): bool { + let value = peel_u8(bcs); + if (value == 0) { + false + } else if (value == 1) { + true + } else { + abort ENotBool + } + } + + /// Read `u8` value from bcs-serialized bytes. + public fun peel_u8(bcs: &mut BCS): u8 { + assert!(v::length(&bcs.bytes) >= 1, EOutOfRange); + v::pop_back(&mut bcs.bytes) + } + + /// Read `u16` value from bcs-serialized bytes. + public fun peel_u16(bcs: &mut BCS): u16 { + assert!(v::length(&bcs.bytes) >= 2, EOutOfRange); + let (value, i) = (0u16, 0u8); + while (i < 16) { + let byte = (v::pop_back(&mut bcs.bytes) as u16); + value = value | (byte << i); + i = i + 8; + }; + value + } + + /// Read `u32` value from bcs-serialized bytes. + public fun peel_u32(bcs: &mut BCS): u32 { + assert!(v::length(&bcs.bytes) >= 4, EOutOfRange); + let (value, i) = (0u32, 0u8); + while (i < 32) { + let byte = (v::pop_back(&mut bcs.bytes) as u32); + value = value | (byte << i); + i = i + 8; + }; + value + } + + /// Read `u64` value from bcs-serialized bytes. + public fun peel_u64(bcs: &mut BCS): u64 { + assert!(v::length(&bcs.bytes) >= 8, EOutOfRange); + let (value, i) = (0u64, 0u8); + while (i < 64) { + let byte = (v::pop_back(&mut bcs.bytes) as u64); + value = value | (byte << i); + i = i + 8; + }; + value + } + + /// Read `u128` value from bcs-serialized bytes. + public fun peel_u128(bcs: &mut BCS): u128 { + assert!(v::length(&bcs.bytes) >= 16, EOutOfRange); + + let (value, i) = (0u128, 0u8); + while (i < 128) { + let byte = (v::pop_back(&mut bcs.bytes) as u128); + value = value | (byte << i); + i = i + 8; + }; + + value + } + + /// Read `u256` value from bcs-serialized bytes. + public fun peel_u256(bcs: &mut BCS): u256 { + assert!(v::length(&bcs.bytes) >= 16, EOutOfRange); + + let (value, i) = (0u256, 0u8); + while (i < 256) { + let byte = (v::pop_back(&mut bcs.bytes) as u256); + value = value | (byte << i); + i = i + 8; + }; + + value + } + + // === Vector === + + /// Read ULEB bytes expecting a vector length. Result should + /// then be used to perform `peel_*` operation LEN times. + /// + /// In BCS `vector` length is implemented with ULEB128; + /// See more here: https://en.wikipedia.org/wiki/LEB128 + public fun peel_vec_length(bcs: &mut BCS): u64 { + let (total, shift, len) = (0u64, 0, 0); + while (true) { + assert!(len <= 4, ELenOutOfRange); + let byte = (v::pop_back(&mut bcs.bytes) as u64); + len = len + 1; + total = total | ((byte & 0x7f) << shift); + if ((byte & 0x80) == 0) { + break + }; + shift = shift + 7; + }; + total + } + + /// Peel a vector of `bool` from serialized bytes. + public fun peel_vec_bool(bcs: &mut BCS): vector { + let (len, i, res) = (peel_vec_length(bcs), 0, vector[]); + while (i < len) { + v::push_back(&mut res, peel_bool(bcs)); + i = i + 1; + }; + res + } + + /// Peel a vector of `u8` (eg string) from serialized bytes. + public fun peel_vec_u8(bcs: &mut BCS): vector { + let (len, i, res) = (peel_vec_length(bcs), 0, vector[]); + while (i < len) { + v::push_back(&mut res, peel_u8(bcs)); + i = i + 1; + }; + res + } + + /// Peel a vector of `u16` (eg string) from serialized bytes. + public fun peel_vec_u16(bcs: &mut BCS): vector { + let (len, i, res) = (peel_vec_length(bcs), 0, vector[]); + while (i < len) { + v::push_back(&mut res, peel_u16(bcs)); + i = i + 1; + }; + res + } + + /// Peel a vector of `u32` (eg string) from serialized bytes. + public fun peel_vec_u32(bcs: &mut BCS): vector { + let (len, i, res) = (peel_vec_length(bcs), 0, vector[]); + while (i < len) { + v::push_back(&mut res, peel_u32(bcs)); + i = i + 1; + }; + res + } + + /// Peel a vector of `u64` from serialized bytes. + public fun peel_vec_u64(bcs: &mut BCS): vector { + let (len, i, res) = (peel_vec_length(bcs), 0, vector[]); + while (i < len) { + v::push_back(&mut res, peel_u64(bcs)); + i = i + 1; + }; + res + } + + /// Peel a vector of `u128` from serialized bytes. + public fun peel_vec_u128(bcs: &mut BCS): vector { + let (len, i, res) = (peel_vec_length(bcs), 0, vector[]); + while (i < len) { + v::push_back(&mut res, peel_u128(bcs)); + i = i + 1; + }; + res + } + + /// Peel a vector of `u256` from serialized bytes. + public fun peel_vec_u256(bcs: &mut BCS): vector { + let (len, i, res) = (peel_vec_length(bcs), 0, vector[]); + while (i < len) { + v::push_back(&mut res, peel_u256(bcs)); + i = i + 1; + }; + res + } + + /// Peel a `vector>` (eg vec of string) from serialized bytes. + public fun peel_vec_vec_u8(bcs: &mut BCS): vector> { + let (len, i, res) = (peel_vec_length(bcs), 0, vector[]); + while (i < len) { + v::push_back(&mut res, peel_vec_u8(bcs)); + i = i + 1; + }; + res + } + + /// Peel a `vector>>` (eg vec of string) from serialized bytes. + public fun peel_vec_vec_vec_u8(bcs: &mut BCS): vector>> { + let (len, i, res) = (peel_vec_length(bcs), 0, vector[]); + while (i < len) { + v::push_back(&mut res, peel_vec_vec_u8(bcs)); + i = i + 1; + }; + res + } + // TODO: re-enable once bit-wise operators in peel_vec_length are supported in the prover + spec module { pragma verify = false; } + + // === Tests === + + #[test_only] + struct Info has drop { a: bool, b: u8, c: u64, d: u128, k: vector, s: address } + + #[test] + #[expected_failure(abort_code = ELenOutOfRange)] + fun test_uleb_len_fail() { + let value = vector[0xff, 0xff, 0xff, 0xff, 0xff]; + let bytes = new(to_bytes(&value)); + let _fail = peel_vec_length(&mut bytes); + abort 2 // TODO: make this test fail + } + + #[test] + #[expected_failure(abort_code = ENotBool)] + fun test_bool_fail() { + let bytes = new(to_bytes(&10u8)); + let _fail = peel_bool(&mut bytes); + } + + #[test] + fun test_bcs() { + + { // boolean: true + let value = true; + let bytes = new(to_bytes(&value)); + assert!(value == peel_bool(&mut bytes), 0); + }; + + { // boolean: false + let value = false; + let bytes = new(to_bytes(&value)); + assert!(value == peel_bool(&mut bytes), 0); + }; + + { // u8 + let value = 100u8; + let bytes = new(to_bytes(&value)); + assert!(value == peel_u8(&mut bytes), 0); + }; + + { // u64 (4 bytes) + let value = 1000100u64; + let bytes = new(to_bytes(&value)); + assert!(value == peel_u64(&mut bytes), 0); + }; + + { // u64 (8 bytes) + let value = 100000000000000u64; + let bytes = new(to_bytes(&value)); + assert!(value == peel_u64(&mut bytes), 0); + }; + + { // u128 (16 bytes) + let value = 100000000000000000000000000u128; + let bytes = new(to_bytes(&value)); + assert!(value == peel_u128(&mut bytes), 0); + }; + + { // vector length + let value = vector[0,0,0,0,0,0,0,0,0,0,0,0,0,0,0]; + let bytes = new(to_bytes(&value)); + assert!(v::length(&value) == peel_vec_length(&mut bytes), 0); + }; + + { // vector length (more data) + let value = vector[ + 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0, + 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0, + 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0, + 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0, + 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0, + 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0 + ]; + + let bytes = new(to_bytes(&value)); + assert!(v::length(&value) == peel_vec_length(&mut bytes), 0); + }; + + { // full deserialization test (ordering) + let info = Info { a: true, b: 100, c: 9999, d: 112333, k: vector[true, false, true, false], s: @0xAAAAAAAAAAA }; + let bytes = new(to_bytes(&info)); + + assert!(info.a == peel_bool(&mut bytes), 0); + assert!(info.b == peel_u8(&mut bytes), 0); + assert!(info.c == peel_u64(&mut bytes), 0); + assert!(info.d == peel_u128(&mut bytes), 0); + + let len = peel_vec_length(&mut bytes); + + assert!(v::length(&info.k) == len, 0); + + }; + + { // read vector of bytes directly + let value = vector[ + vector[1,2,3,4,5], + vector[1,2,3,4,5], + vector[1,2,3,4,5] + ]; + let bytes = new(to_bytes(&value)); + assert!(value == peel_vec_vec_u8(&mut bytes), 0); + }; + + { // read vector of bytes directly + let value = vector[1,2,3,4,5]; + let bytes = new(to_bytes(&value)); + assert!(value == peel_vec_u8(&mut bytes), 0); + }; + + { // read vector of bytes directly + let value = vector[1,2,3,4,5]; + let bytes = new(to_bytes(&value)); + assert!(value == peel_vec_u64(&mut bytes), 0); + }; + + { // read vector of bytes directly + let value = vector[1,2,3,4,5]; + let bytes = new(to_bytes(&value)); + assert!(value == peel_vec_u128(&mut bytes), 0); + }; + + { // read vector of bytes directly + let value = vector[true, false, true, false]; + let bytes = new(to_bytes(&value)); + assert!(value == peel_vec_bool(&mut bytes), 0); + }; + + } +} \ No newline at end of file From 072f79678fe8808bb5cb8e1046cdc33381c11aa7 Mon Sep 17 00:00:00 2001 From: Dhaval Purohit Date: Tue, 30 Sep 2025 13:03:59 +0530 Subject: [PATCH 02/14] WIP: Resolve comments and added further implementation --- .../supra-framework/sources/block.move | 3 + .../configs/leader_ban_registry_config.move | 40 +++- .../sources/leader_ban_registry.move | 193 ++++++++++++++---- .../supra-framework/sources/stake.move | 40 ++++ 4 files changed, 229 insertions(+), 47 deletions(-) diff --git a/aptos-move/framework/supra-framework/sources/block.move b/aptos-move/framework/supra-framework/sources/block.move index 07994454624fd..40f1f41f3a1f8 100644 --- a/aptos-move/framework/supra-framework/sources/block.move +++ b/aptos-move/framework/supra-framework/sources/block.move @@ -18,6 +18,7 @@ module supra_framework::block { use supra_framework::system_addresses; use supra_framework::timestamp; use supra_framework::transaction_fee; + use supra_framework::leader_ban_registry; friend supra_framework::genesis; @@ -219,6 +220,8 @@ module supra_framework::block { // transition is the last block in the previous epoch. stake::update_performance_statistics(proposer_index, failed_proposer_indices); state_storage::on_new_block(reconfiguration::current_epoch()); + + leader_ban_registry::update_ban_registry(epoch, round, proposer_index, failed_proposer_indices); block_metadata_ref.epoch_interval } diff --git a/aptos-move/framework/supra-framework/sources/configs/leader_ban_registry_config.move b/aptos-move/framework/supra-framework/sources/configs/leader_ban_registry_config.move index a76433772c96b..6c459eb9b263d 100644 --- a/aptos-move/framework/supra-framework/sources/configs/leader_ban_registry_config.move +++ b/aptos-move/framework/supra-framework/sources/configs/leader_ban_registry_config.move @@ -65,39 +65,67 @@ module supra_framework::leader_ban_registry_config { } } + #[view] + public fun get_ban_registry_param(): (vector, u8) acquires BanRegistryParameters { + if (!exists::(@supra_framework)) { + let ban_registry_config = borrow_global(@supra_framework); + return ( + ban_registry_config.config, + ban_registry_config.version + ) + }; + (vector::empty(), 0) + } + + #[view] + public fun get_ban_registry_param_v0(): (u8, u32, u8) acquires BanRegistryParameters { + if (!exists::(@supra_framework)) { + let ban_registry_config = borrow_global(@supra_framework); + if (ban_registry_config.version == 0) { + let ban_registry_params = deserialise_v0_params(ban_registry_config.config); + return ( + ban_registry_params.initial_elections_denied, + ban_registry_params.max_elections_denied, + ban_registry_params.minimum_unbanned_proposers + ) + } + }; + (0, 0, 0) + } + /// Provide initial election denied value efficiently /// by decdoing correct version and if not set then 0 public fun get_initial_election_denied(): u8 acquires BanRegistryParameters { if (!exists::(@supra_framework)) { let ban_registry_config = borrow_global(@supra_framework); if (ban_registry_config.version == 0) { - let ban_registry_params = deserialise_v1_param(ban_registry_config.config); + let ban_registry_params = deserialise_v0_params(ban_registry_config.config); return ban_registry_params.initial_elections_denied; } }; 0 } - /// Provide max electoion denied value efficiently + /// Provide max election denied value efficiently /// by decdoing correct version and if not set then 0 public fun get_max_elections_denied(): u32 acquires BanRegistryParameters { if (!exists::(@supra_framework)) { let ban_registry_config = borrow_global(@supra_framework); if (ban_registry_config.version == 0) { - let ban_registry_params = deserialise_v1_param(ban_registry_config.config); + let ban_registry_params = deserialise_v0_params(ban_registry_config.config); return ban_registry_params.max_elections_denied; } }; 0 } - /// Provide max electoion denied value efficiently + /// Provide max election denied value efficiently /// by decdoing correct version and if not set then 0 public fun get_minimum_unbanned_proposers(): u8 acquires BanRegistryParameters { if (!exists::(@supra_framework)) { let ban_registry_config = borrow_global(@supra_framework); if (ban_registry_config.version == 0) { - let ban_registry_params = deserialise_v1_param(ban_registry_config.config); + let ban_registry_params = deserialise_v0_params(ban_registry_config.config); return ban_registry_params.minimum_unbanned_proposers; } }; @@ -105,7 +133,7 @@ module supra_framework::leader_ban_registry_config { } /// Decoding bytes to `BanRegistryParametersV0` using bcs - fun deserialise_v1_param(bytes: vector): BanRegistryParametersV0 { + fun deserialise_v0_params(bytes: vector): BanRegistryParametersV0 { let bcs_bytes = decode_bcs::new(bytes); let initial_elections_denied: u8 = decode_bcs::peel_u8(&mut bcs_bytes); let max_elections_denied: u32 = decode_bcs::peel_u32(&mut bcs_bytes); diff --git a/aptos-move/framework/supra-framework/sources/leader_ban_registry.move b/aptos-move/framework/supra-framework/sources/leader_ban_registry.move index db517796ca3e4..e8816ced56188 100644 --- a/aptos-move/framework/supra-framework/sources/leader_ban_registry.move +++ b/aptos-move/framework/supra-framework/sources/leader_ban_registry.move @@ -1,17 +1,21 @@ /// Maintains the list of banned validators and updates counters on every epoch module supra_framework::leader_ban_registry { use std::error; - use supra_std::enumerable_map; + use std::option; + use std::option::Option; use supra_framework::system_addresses; + use std::vector; + use supra_framework::stake; + use supra_framework::leader_ban_registry_config; + + friend supra_framework::block; /// Leader ban registry already initialized const EBAN_REGISTRY_ALREADY_EXISTS: u64 = 1; /// Leader ban registry not initialized const EBAN_REGISTRY_NOT_INITIALIZED: u64 = 2; - /// Validator already exists in registry - const EVALIDATOR_ALREADY_EXISTS: u64 = 3; - /// Validator not exists in registry - const EVALIDATOR_DOESNT_EXISTS: u64 = 4; + /// Latest view already initialized + const ELATEST_VIEW_ALREADY_EXISTS: u64 = 3; /// Holds metrics for banned round, epoch and round server in prev epoch struct ActiveBan has store, drop { @@ -24,12 +28,33 @@ module supra_framework::leader_ban_registry { struct ValidatorBansWithAddress has store, drop { active: ActiveBan, consecutive_bans: u32, - pool_address: address, + pool_address: address } /// Holds ban registry struct BanRegistry has drop, store, key { - bans: enumerable_map::EnumerableMap + bans: vector + } + + /// Holds latest processed round and epoch + struct LatestView has drop, store, key { + epoch: u64, + round: u64 + } + + #[event] + struct BanAdded has drop { + pool_address: address, + epoch: u64, + round: u64, + consecutive_count: u64 + } + + #[event] + struct BanRemoved has drop { + pool_address: address, + epoch: u64, + round: u64, } /// Initialise leader ban registry @@ -39,56 +64,142 @@ module supra_framework::leader_ban_registry { !exists(@supra_framework), error::already_exists(EBAN_REGISTRY_ALREADY_EXISTS) ); - move_to(supra_framework, BanRegistry { bans: enumerable_map::new_map() }); + assert!( + !exists(@supra_framework), + error::already_exists(EBAN_REGISTRY_ALREADY_EXISTS) + ); + move_to(supra_framework, BanRegistry { bans: vector::empty() }); } - /// Adding an entry for new validator - public entry fun add_new_ban_entry( - supra_framework: &signer, validator_address: address, pool_address: address, epoch_earned: u64, round_earned: u64 + #[view] + public fun get_ban_registry() : vector acquires BanRegistry { + if (!exists(@supra_framework)) { + return (vector::empty()); + }; + let ban_registry = borrow_global(@supra_framework); + ban_registry.bans + } + + #[view] + public fun get_initial_ban_duration(): u64 { + let initail_election_denied = leader_ban_registry_config::get_initial_election_denied() as u64; + let committee_size = stake::get_committe_size(); + committee_size * initail_election_denied + } + + #[view] + public fun get_max_ban_duration(): u64 { + let max_ban_election_denied = leader_ban_registry_config::get_max_elections_denied() as u64; + let committee_size = stake::get_committe_size(); + committee_size * max_ban_election_denied + } + + /// Add or update the ban registry as per block metadata + public(friend) fun update_ban_registry( + epoch_earned: u64, round_earned: u64, proposer_index: Option, failed_proposer_indices: vector ) - acquires BanRegistry + acquires BanRegistry, LatestView { - // TODO: epoch can be retrived from reconfigure - // TODO: what about round - // TODO: Validator address and index can be fetched pool address see stake.move - - system_addresses::assert_supra_framework(supra_framework); - assert_registry_initialized(); + if (!exists(@supra_framework)) { + return ; + }; + if (!exists(@supra_framework)) { + return ; + }; let ban_registry = borrow_global_mut(@supra_framework); + let latest_view = borrow_global_mut(@supra_framework); + latest_view.epoch = epoch_earned; + latest_view.round = round_earned; + + // ban the failed proposer indices + ban(epoch_earned, round_earned, failed_proposer_indices, ban_registry); + + // unban the proposer + if (std::option::is_some(&proposer_index)) { + let extracted_proposer_index = *std::option::borrow(&mut proposer_index); + unban(epoch_earned, round_earned, extracted_proposer_index, ban_registry); + }; + } - if (enumerable_map::contains(&ban_registry.bans, validator_address)) { - abort error::already_exists(EVALIDATOR_ALREADY_EXISTS) + /// Adds failed proposer indices to ban registry + fun ban(epoch_earned: u64, round_earned: u64, failed_proposer_indices: vector, ban_registry: &mut BanRegistry) + { + let initial_ban_duration = get_initial_ban_duration(); + if (initial_ban_duration == 0) { + return; }; - enumerable_map::add_value(&mut ban_registry.bans, validator_address, ValidatorBansWithAddress { - active: ActiveBan { - epoch_earned, - round_earned, - rounds_served_in_previous_epochs: 0 - }, - consecutive_bans:0, - pool_address - }) + vector::for_each(failed_proposer_indices, |failed_validator_index| { + let validator_pool_address_opt= stake::get_pool_address_from_index(failed_validator_index); + if (option::is_some(&validator_pool_address_opt)) { + let validator_pool_address = option::extract(&mut validator_pool_address_opt); + let (is_exists, index) = is_pool_exists(&ban_registry.bans, validator_pool_address); + if (is_exists) { + let bans = vector::borrow_mut(&mut ban_registry.bans, index); + bans.consecutive_bans += 1; + // TODO: we can update the registry here if max duration already passed + } else { + vector::push_back(&mut ban_registry.bans, ValidatorBansWithAddress { + active: ActiveBan { + epoch_earned, + round_earned, + rounds_served_in_previous_epochs: 0 + }, + consecutive_bans: 0, + pool_address: validator_pool_address + }); + }; + }; + }); } - /// Remvoing an entry of validator from registry - public entry fun remove_ban_entry(supra_framework: &signer, validator_address: address) - acquires BanRegistry + /// Remvoing an entry of validator from registry if found at proposer index + fun unban(epoch_earned: u64, round_earned: u64, proposer_index: u64, ban_registry: &mut BanRegistry) { - system_addresses::assert_supra_framework(supra_framework); - assert_registry_initialized(); - let ban_registry = borrow_global_mut(@supra_framework); + let validator_pool_address_opt= stake::get_pool_address_from_index(proposer_index); + if (option::is_some(&validator_pool_address_opt)) { + let validator_pool_address = option::extract(&mut validator_pool_address_opt); + let (exists, index) = is_pool_exists(&ban_registry.bans, validator_pool_address); + if (exists) { + vector::swap_remove(&mut ban_registry.bans, index); + }; + }; + } - if (!enumerable_map::contains(&ban_registry.bans, validator_address)) { - abort error::not_found(EVALIDATOR_DOESNT_EXISTS) + /// Update counts on every epoch + public(friend) fun on_new_epoch() acquires BanRegistry,LatestView { + if (!exists(@supra_framework)) { + return; }; + if (!exists(@supra_framework)) { + return; + }; + let latest_view = borrow_global(@supra_framework); + let ban_registry = borrow_global_mut(@supra_framework); + + vector::for_each_mut(&mut ban_registry.bans, |v| { + if (has_started(&v.active, latest_view)) { + if (latest_view.epoch > v.active.epoch_earned) { + v.active.rounds_served_in_previous_epochs += latest_view.round; + } else { + v.active.rounds_served_in_previous_epochs += latest_view.round - v.active.round_earned; + } + }; + }); + // TODO: Do we need update the list here ? for those entreis which max duration already passed? + } - enumerable_map::remove_value(&mut ban_registry.bans, validator_address) + /// Checks whether pool entry exists or not + fun is_pool_exists(bans: &vector, addr: address) : (bool, u64) { + let (is_exist, index) = vector::find(bans, |v| { + v.pool_address == addr + }); + (is_exist, index) } - /// Update countes on every epoch - public(friend) fun on_new_epoch() { - /// TODO: What are the use cases configuration here? + /// Checks if ban has already started + fun has_started(active_ban: &ActiveBan, latest_view: &LatestView): bool { + active_ban.epoch_earned <= latest_view.epoch && active_ban.round_earned < latest_view.round } /// Validates registry initialised if not aborted with `EBAN_REGISTRY_NOT_INITIALIZED` diff --git a/aptos-move/framework/supra-framework/sources/stake.move b/aptos-move/framework/supra-framework/sources/stake.move index 4b6d9451c2205..bfe56967a236c 100644 --- a/aptos-move/framework/supra-framework/sources/stake.move +++ b/aptos-move/framework/supra-framework/sources/stake.move @@ -40,6 +40,7 @@ module supra_framework::stake { friend supra_framework::reconfiguration; friend supra_framework::reconfiguration_with_dkg; friend supra_framework::transaction_fee; + friend supra_framework::leader_ban_registry; /// Validator Config not published. const EVALIDATOR_CONFIG: u64 = 1; @@ -506,6 +507,45 @@ module supra_framework::stake { move_to(supra_framework, SupraCoinCapabilities { mint_cap }) } + /// To get validator pool address from validator index + public(friend) fun get_pool_address_from_index(index: u64) : Option
+ acquires ValidatorSet + { + if (exists(@supra_framework)) { + let validator_set = borrow_global(@supra_framework); + // it can happen that some validator may added a leave request in between + // this can change the order of indexes in active validator + // so need to iterate through all until find pool address in active and pending_active + let return_addr = option::none(); + vector::for_each_ref(&validator_set.active_validators, |validator_info| { + if (index == validator_info.config.validator_index) { + return_addr = option::some(validator_info.addr); + } + }); + vector::for_each_ref(&validator_set.pending_active, |validator_info| { + if (index == validator_info.config.validator_index) { + return_addr = option::some(validator_info.addr); + } + }); + // as we are sure that either the index can be found in active or pending active or none + // so not returning the address in between but at the end. + return return_addr; + }; + option::none() + } + + /// Returns committee size + public(friend) fun get_committe_size() : u64 + acquires ValidatorSet + { + let commitee_size= 0; + if (exists(@supra_framework)) { + let validator_set = borrow_global(@supra_framework); + commitee_size = vector::length(&validator_set.active_validators) + vector::length(&validator_set.pending_active); + }; + commitee_size + } + /// Allow on chain governance to remove validators from the validator set. public fun remove_validators( supra_framework: &signer, From 22e17c5454904f66c890f34371fbdde156d585f2 Mon Sep 17 00:00:00 2001 From: Dhaval Purohit Date: Fri, 3 Oct 2025 13:23:54 +0530 Subject: [PATCH 03/14] resolved comments --- .../configs/leader_ban_registry_config.move | 76 ++++++---- .../sources/leader_ban_registry.move | 138 ++++++++++++++---- .../supra-framework/sources/stake.move | 32 +++- 3 files changed, 184 insertions(+), 62 deletions(-) diff --git a/aptos-move/framework/supra-framework/sources/configs/leader_ban_registry_config.move b/aptos-move/framework/supra-framework/sources/configs/leader_ban_registry_config.move index 6c459eb9b263d..b6d6381ae24d9 100644 --- a/aptos-move/framework/supra-framework/sources/configs/leader_ban_registry_config.move +++ b/aptos-move/framework/supra-framework/sources/configs/leader_ban_registry_config.move @@ -1,6 +1,8 @@ /// Provides the config related to leader ban registry module supra_framework::leader_ban_registry_config { use std::error; + use std::option; + use std::option::Option; use std::vector; use supra_std::decode_bcs; use supra_framework::config_buffer; @@ -15,6 +17,8 @@ module supra_framework::leader_ban_registry_config { const EINVALID_VERSION: u64 = 2; /// Decoding from version bytes failed const EINVALID_VERSION_BYTES: u64 = 3; + /// The BanRegistryParameters already initialised + const EALREADY_INITIALISED: u64 = 4; struct BanRegistryParameters has drop, key, store { @@ -22,7 +26,7 @@ module supra_framework::leader_ban_registry_config { version: u8 } - struct BanRegistryParametersV0 has drop { + struct BanRegistryParametersV0 has drop, key, store { initial_elections_denied: u8, max_elections_denied: u32, minimum_unbanned_proposers: u8 @@ -32,8 +36,16 @@ module supra_framework::leader_ban_registry_config { public(friend) fun initialize(supra_framework: &signer, config: vector) { system_addresses::assert_supra_framework(supra_framework); assert!(vector::length(&config) != 0, error::invalid_argument(EINVALID_CONFIG)); + assert!(!exists(@supra_framework), error::already_exists(EALREADY_INITIALISED)); + let v0 = deserialise_v0_params(config); + if (option::is_none(&v0)) { + abort error::invalid_argument(EINVALID_VERSION_BYTES); + }; // we always init with version 0 move_to(supra_framework, BanRegistryParameters { config, version: 0 }); + let v0_params = option::extract(&mut v0); + move_to(supra_framework, v0_params); + } /// This can be called by on-chain governance to update on-chain configs for the next epoch. @@ -53,7 +65,7 @@ module supra_framework::leader_ban_registry_config { } /// Only used in reconfigurations to apply the pending `BanRegistryParameters`, if there is any. - public(friend) fun on_new_epoch(framework: &signer) acquires BanRegistryParameters { + public(friend) fun on_new_epoch(framework: &signer) acquires BanRegistryParameters, BanRegistryParametersV0 { system_addresses::assert_supra_framework(framework); if (config_buffer::does_exist()) { let new_config = config_buffer::extract(); @@ -62,6 +74,20 @@ module supra_framework::leader_ban_registry_config { } else { move_to(framework, new_config); }; + let params = borrow_global(@supra_framework); + if (params.version == 0) { + let v0 = deserialise_v0_params(params.config); + // This is to prevent abort on new epoch if deserialise failed. + if (option::is_some(&v0)){ + let ban_registry_params = option::extract(&mut v0); + if (exists(@supra_framework)) { + *borrow_global_mut(@supra_framework) = ban_registry_params; + } else { + move_to(framework, ban_registry_params); + } + } + } + // later version can be assigned here } } @@ -78,11 +104,11 @@ module supra_framework::leader_ban_registry_config { } #[view] - public fun get_ban_registry_param_v0(): (u8, u32, u8) acquires BanRegistryParameters { + public fun get_ban_registry_param_v0(): (u8, u32, u8) acquires BanRegistryParameters, BanRegistryParametersV0 { if (!exists::(@supra_framework)) { let ban_registry_config = borrow_global(@supra_framework); if (ban_registry_config.version == 0) { - let ban_registry_params = deserialise_v0_params(ban_registry_config.config); + let ban_registry_params = borrow_global(@supra_framework); return ( ban_registry_params.initial_elections_denied, ban_registry_params.max_elections_denied, @@ -93,39 +119,36 @@ module supra_framework::leader_ban_registry_config { (0, 0, 0) } - /// Provide initial election denied value efficiently - /// by decdoing correct version and if not set then 0 - public fun get_initial_election_denied(): u8 acquires BanRegistryParameters { + /// Provide initial election denied value + public fun get_initial_elections_denied(): u8 acquires BanRegistryParameters, BanRegistryParametersV0 { if (!exists::(@supra_framework)) { let ban_registry_config = borrow_global(@supra_framework); if (ban_registry_config.version == 0) { - let ban_registry_params = deserialise_v0_params(ban_registry_config.config); + let ban_registry_params = borrow_global(@supra_framework); return ban_registry_params.initial_elections_denied; } }; 0 } - /// Provide max election denied value efficiently - /// by decdoing correct version and if not set then 0 - public fun get_max_elections_denied(): u32 acquires BanRegistryParameters { + /// Provide max election denied value + public fun get_max_elections_denied(): u32 acquires BanRegistryParameters, BanRegistryParametersV0 { if (!exists::(@supra_framework)) { let ban_registry_config = borrow_global(@supra_framework); if (ban_registry_config.version == 0) { - let ban_registry_params = deserialise_v0_params(ban_registry_config.config); + let ban_registry_params = borrow_global(@supra_framework); return ban_registry_params.max_elections_denied; } }; 0 } - /// Provide max election denied value efficiently - /// by decdoing correct version and if not set then 0 - public fun get_minimum_unbanned_proposers(): u8 acquires BanRegistryParameters { + /// Provide minimum unbanned proposers value + public fun get_minimum_unbanned_proposers(): u8 acquires BanRegistryParameters, BanRegistryParametersV0 { if (!exists::(@supra_framework)) { let ban_registry_config = borrow_global(@supra_framework); if (ban_registry_config.version == 0) { - let ban_registry_params = deserialise_v0_params(ban_registry_config.config); + let ban_registry_params = borrow_global(@supra_framework); return ban_registry_params.minimum_unbanned_proposers; } }; @@ -133,20 +156,19 @@ module supra_framework::leader_ban_registry_config { } /// Decoding bytes to `BanRegistryParametersV0` using bcs - fun deserialise_v0_params(bytes: vector): BanRegistryParametersV0 { + fun deserialise_v0_params(bytes: vector): Option { let bcs_bytes = decode_bcs::new(bytes); let initial_elections_denied: u8 = decode_bcs::peel_u8(&mut bcs_bytes); let max_elections_denied: u32 = decode_bcs::peel_u32(&mut bcs_bytes); let minimum_unbanned_proposers: u8 = decode_bcs::peel_u8(&mut bcs_bytes); - // making sure not bytes left to decode means correct parameter version - assert!( - vector::length(&decode_bcs::into_remainder_bytes(bcs_bytes)) == 0, - error::out_of_range(EINVALID_VERSION_BYTES) - ); - BanRegistryParametersV0 { - initial_elections_denied, - max_elections_denied, - minimum_unbanned_proposers - } + // making sure no bytes left to decode means correct parameter version + if (vector::length(&decode_bcs::into_remainder_bytes(bcs_bytes)) == 0) { + return option::some(BanRegistryParametersV0 { + initial_elections_denied, + max_elections_denied, + minimum_unbanned_proposers + }) + }; + option::none() } } diff --git a/aptos-move/framework/supra-framework/sources/leader_ban_registry.move b/aptos-move/framework/supra-framework/sources/leader_ban_registry.move index e8816ced56188..4f33086e83533 100644 --- a/aptos-move/framework/supra-framework/sources/leader_ban_registry.move +++ b/aptos-move/framework/supra-framework/sources/leader_ban_registry.move @@ -1,10 +1,13 @@ /// Maintains the list of banned validators and updates counters on every epoch module supra_framework::leader_ban_registry { use std::error; + use std::features; use std::option; use std::option::Option; use supra_framework::system_addresses; use std::vector; + use aptos_std::math64::{pow, min}; + use supra_framework::event; use supra_framework::stake; use supra_framework::leader_ban_registry_config; @@ -44,17 +47,24 @@ module supra_framework::leader_ban_registry { #[event] struct BanAdded has drop { + pool_address: address, + epoch: u64, + round: u64 + } + + #[event] + struct BanUpdated has drop { pool_address: address, epoch: u64, round: u64, - consecutive_count: u64 + consecutive_bans: u32 } #[event] struct BanRemoved has drop { pool_address: address, epoch: u64, - round: u64, + round: u64 } /// Initialise leader ban registry @@ -69,6 +79,7 @@ module supra_framework::leader_ban_registry { error::already_exists(EBAN_REGISTRY_ALREADY_EXISTS) ); move_to(supra_framework, BanRegistry { bans: vector::empty() }); + move_to(supra_framework, LatestView { epoch: 0, round: 0 }); } #[view] @@ -82,16 +93,16 @@ module supra_framework::leader_ban_registry { #[view] public fun get_initial_ban_duration(): u64 { - let initail_election_denied = leader_ban_registry_config::get_initial_election_denied() as u64; - let committee_size = stake::get_committe_size(); - committee_size * initail_election_denied + let initial_elections_denied = leader_ban_registry_config::get_initial_elections_denied() as u64; + let committee_size = stake::get_committee_size(); + committee_size * initial_elections_denied } #[view] public fun get_max_ban_duration(): u64 { - let max_ban_election_denied = leader_ban_registry_config::get_max_elections_denied() as u64; - let committee_size = stake::get_committe_size(); - committee_size * max_ban_election_denied + let max_elections_denied = leader_ban_registry_config::get_max_elections_denied() as u64; + let committee_size = stake::get_committee_size(); + committee_size * max_elections_denied } /// Add or update the ban registry as per block metadata @@ -130,24 +141,55 @@ module supra_framework::leader_ban_registry { }; vector::for_each(failed_proposer_indices, |failed_validator_index| { - let validator_pool_address_opt= stake::get_pool_address_from_index(failed_validator_index); + let validator_pool_address_opt = stake::get_pool_address_from_index(failed_validator_index); if (option::is_some(&validator_pool_address_opt)) { let validator_pool_address = option::extract(&mut validator_pool_address_opt); - let (is_exists, index) = is_pool_exists(&ban_registry.bans, validator_pool_address); - if (is_exists) { + let (is_banned, index) = vector::find(&ban_registry.bans, |v| { + validator_pool_address == v.pool_address + }); + if (is_banned) { let bans = vector::borrow_mut(&mut ban_registry.bans, index); bans.consecutive_bans += 1; - // TODO: we can update the registry here if max duration already passed + if (duration(bans) >= get_max_ban_duration()) { + vector::swap_remove(&mut ban_registry.bans, index); + if (features::module_event_enabled()) { + event::emit(BanRemoved { + pool_address: validator_pool_address, + epoch: epoch_earned, + round: round_earned + }); + } + } else { + if (features::module_event_enabled()) { + event::emit(BanUpdated { + pool_address: validator_pool_address, + epoch: epoch_earned, + round: round_earned, + consecutive_bans: bans.consecutive_bans + }); + } + } } else { - vector::push_back(&mut ban_registry.bans, ValidatorBansWithAddress { - active: ActiveBan { - epoch_earned, - round_earned, - rounds_served_in_previous_epochs: 0 - }, - consecutive_bans: 0, - pool_address: validator_pool_address - }); + let ban_registry_len = vector::length(&ban_registry.bans); + if (can_be_added_in_ban(ban_registry_len)) { + vector::push_back(&mut ban_registry.bans, ValidatorBansWithAddress { + active: ActiveBan { + epoch_earned, + round_earned, + rounds_served_in_previous_epochs: 0 + }, + consecutive_bans: 0, + pool_address: validator_pool_address + }); + + if (features::module_event_enabled()) { + event::emit(BanAdded { + pool_address: validator_pool_address, + epoch: epoch_earned, + round: round_earned + }); + } + } }; }; }); @@ -159,8 +201,10 @@ module supra_framework::leader_ban_registry { let validator_pool_address_opt= stake::get_pool_address_from_index(proposer_index); if (option::is_some(&validator_pool_address_opt)) { let validator_pool_address = option::extract(&mut validator_pool_address_opt); - let (exists, index) = is_pool_exists(&ban_registry.bans, validator_pool_address); - if (exists) { + let (is_banned, index) = vector::find(&ban_registry.bans, |v| { + validator_pool_address == v.pool_address + }); + if (is_banned) { vector::swap_remove(&mut ban_registry.bans, index); }; }; @@ -177,6 +221,8 @@ module supra_framework::leader_ban_registry { let latest_view = borrow_global(@supra_framework); let ban_registry = borrow_global_mut(@supra_framework); + let updated_pool_addresses = stake::get_committee_pool_addresses(); + let pool_addresses_to_remove = vector::empty(); vector::for_each_mut(&mut ban_registry.bans, |v| { if (has_started(&v.active, latest_view)) { if (latest_view.epoch > v.active.epoch_earned) { @@ -185,16 +231,24 @@ module supra_framework::leader_ban_registry { v.active.rounds_served_in_previous_epochs += latest_view.round - v.active.round_earned; } }; + if (vector::contains(&updated_pool_addresses, &v.pool_address)) { + vector::push_back(&mut pool_addresses_to_remove, v.pool_address) + } }); - // TODO: Do we need update the list here ? for those entreis which max duration already passed? - } - /// Checks whether pool entry exists or not - fun is_pool_exists(bans: &vector, addr: address) : (bool, u64) { - let (is_exist, index) = vector::find(bans, |v| { - v.pool_address == addr + vector::for_each(pool_addresses_to_remove, |p| { + let (is_exist, index) = vector::find(&mut ban_registry.bans, |v|{ v.pool_address == p }); + if (is_exist) { + vector::swap_remove(&mut ban_registry.bans, index); + if (features::module_event_enabled()) { + event::emit(BanRemoved { + pool_address: p, + epoch: latest_view.epoch, + round: latest_view.round + }); + } + } }); - (is_exist, index) } /// Checks if ban has already started @@ -202,6 +256,30 @@ module supra_framework::leader_ban_registry { active_ban.epoch_earned <= latest_view.epoch && active_ban.round_earned < latest_view.round } + /// Calculate the duration + fun duration(ban: &ValidatorBansWithAddress): u64 { + let initial_ban_duration = get_initial_ban_duration(); + let max_ban_duration = get_max_ban_duration(); + let duration = initial_ban_duration * pow(2, ban.consecutive_bans as u64); + if (duration >= ban.active.rounds_served_in_previous_epochs) { + duration = duration - ban.active.rounds_served_in_previous_epochs; + } else { + duration = 0; + }; + min(duration, max_ban_duration) + } + + fun can_be_added_in_ban(ban_registry_len: u64) : bool { + let minimum_unbanned_proposer = leader_ban_registry_config::get_minimum_unbanned_proposers() as u64; + let committee_size = stake::get_committee_size(); + if (committee_size > ban_registry_len) { + if (committee_size - ban_registry_len > minimum_unbanned_proposer) { + return true; + }; + }; + false + } + /// Validates registry initialised if not aborted with `EBAN_REGISTRY_NOT_INITIALIZED` fun assert_registry_initialized() { assert!( diff --git a/aptos-move/framework/supra-framework/sources/stake.move b/aptos-move/framework/supra-framework/sources/stake.move index bfe56967a236c..fabe920e7bc64 100644 --- a/aptos-move/framework/supra-framework/sources/stake.move +++ b/aptos-move/framework/supra-framework/sources/stake.move @@ -520,32 +520,54 @@ module supra_framework::stake { vector::for_each_ref(&validator_set.active_validators, |validator_info| { if (index == validator_info.config.validator_index) { return_addr = option::some(validator_info.addr); + break; } }); - vector::for_each_ref(&validator_set.pending_active, |validator_info| { + // as we are sure that either the index can be found in active or pending inactive or none + // returning early without looping through pending_inactive if found + if (option::is_some(&return_addr)) { + return return_addr; + }; + vector::for_each_ref(&validator_set.pending_inactive, |validator_info| { if (index == validator_info.config.validator_index) { return_addr = option::some(validator_info.addr); + break; } }); - // as we are sure that either the index can be found in active or pending active or none - // so not returning the address in between but at the end. return return_addr; }; option::none() } /// Returns committee size - public(friend) fun get_committe_size() : u64 + public(friend) fun get_committee_size() : u64 acquires ValidatorSet { let commitee_size= 0; if (exists(@supra_framework)) { let validator_set = borrow_global(@supra_framework); - commitee_size = vector::length(&validator_set.active_validators) + vector::length(&validator_set.pending_active); + commitee_size = vector::length(&validator_set.active_validators) + vector::length(&validator_set.pending_inactive); }; commitee_size } + /// Returns committee size + public(friend) fun get_committee_pool_addresses() : vector
+ acquires ValidatorSet + { + let pool_addresses= vector::empty(); + if (exists(@supra_framework)) { + let validator_set = borrow_global(@supra_framework); + vector::for_each_ref(&validator_set.active_validators, |validator_info| { + vector::push_back(&mut pool_addresses, validator_info.addr); + }); + vector::for_each_ref(&validator_set.pending_inactive, |validator_info| { + vector::push_back(&mut pool_addresses, validator_info.addr); + }); + }; + pool_addresses + } + /// Allow on chain governance to remove validators from the validator set. public fun remove_validators( supra_framework: &signer, From 477255088a125f1037210510fad929e5bd3b6253 Mon Sep 17 00:00:00 2001 From: Dhaval Purohit Date: Fri, 3 Oct 2025 14:00:49 +0530 Subject: [PATCH 04/14] Fixed compilation issue --- .../configs/leader_ban_registry_config.move | 10 +++++----- .../sources/leader_ban_registry.move | 16 ++++++++-------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/aptos-move/framework/supra-framework/sources/configs/leader_ban_registry_config.move b/aptos-move/framework/supra-framework/sources/configs/leader_ban_registry_config.move index b6d6381ae24d9..4bad156af0572 100644 --- a/aptos-move/framework/supra-framework/sources/configs/leader_ban_registry_config.move +++ b/aptos-move/framework/supra-framework/sources/configs/leader_ban_registry_config.move @@ -93,7 +93,7 @@ module supra_framework::leader_ban_registry_config { #[view] public fun get_ban_registry_param(): (vector, u8) acquires BanRegistryParameters { - if (!exists::(@supra_framework)) { + if (!exists(@supra_framework)) { let ban_registry_config = borrow_global(@supra_framework); return ( ban_registry_config.config, @@ -105,7 +105,7 @@ module supra_framework::leader_ban_registry_config { #[view] public fun get_ban_registry_param_v0(): (u8, u32, u8) acquires BanRegistryParameters, BanRegistryParametersV0 { - if (!exists::(@supra_framework)) { + if (!exists(@supra_framework)) { let ban_registry_config = borrow_global(@supra_framework); if (ban_registry_config.version == 0) { let ban_registry_params = borrow_global(@supra_framework); @@ -121,7 +121,7 @@ module supra_framework::leader_ban_registry_config { /// Provide initial election denied value public fun get_initial_elections_denied(): u8 acquires BanRegistryParameters, BanRegistryParametersV0 { - if (!exists::(@supra_framework)) { + if (!exists(@supra_framework)) { let ban_registry_config = borrow_global(@supra_framework); if (ban_registry_config.version == 0) { let ban_registry_params = borrow_global(@supra_framework); @@ -133,7 +133,7 @@ module supra_framework::leader_ban_registry_config { /// Provide max election denied value public fun get_max_elections_denied(): u32 acquires BanRegistryParameters, BanRegistryParametersV0 { - if (!exists::(@supra_framework)) { + if (!exists(@supra_framework)) { let ban_registry_config = borrow_global(@supra_framework); if (ban_registry_config.version == 0) { let ban_registry_params = borrow_global(@supra_framework); @@ -145,7 +145,7 @@ module supra_framework::leader_ban_registry_config { /// Provide minimum unbanned proposers value public fun get_minimum_unbanned_proposers(): u8 acquires BanRegistryParameters, BanRegistryParametersV0 { - if (!exists::(@supra_framework)) { + if (!exists(@supra_framework)) { let ban_registry_config = borrow_global(@supra_framework); if (ban_registry_config.version == 0) { let ban_registry_params = borrow_global(@supra_framework); diff --git a/aptos-move/framework/supra-framework/sources/leader_ban_registry.move b/aptos-move/framework/supra-framework/sources/leader_ban_registry.move index 4f33086e83533..c7697c38c9a7a 100644 --- a/aptos-move/framework/supra-framework/sources/leader_ban_registry.move +++ b/aptos-move/framework/supra-framework/sources/leader_ban_registry.move @@ -46,14 +46,14 @@ module supra_framework::leader_ban_registry { } #[event] - struct BanAdded has drop { + struct BanAdded has drop, store { pool_address: address, epoch: u64, round: u64 } #[event] - struct BanUpdated has drop { + struct BanUpdated has drop, store { pool_address: address, epoch: u64, round: u64, @@ -61,7 +61,7 @@ module supra_framework::leader_ban_registry { } #[event] - struct BanRemoved has drop { + struct BanRemoved has drop, store { pool_address: address, epoch: u64, round: u64 @@ -93,9 +93,9 @@ module supra_framework::leader_ban_registry { #[view] public fun get_initial_ban_duration(): u64 { - let initial_elections_denied = leader_ban_registry_config::get_initial_elections_denied() as u64; + let initial_elections_denied = leader_ban_registry_config::get_initial_elections_denied(); let committee_size = stake::get_committee_size(); - committee_size * initial_elections_denied + committee_size * (initial_elections_denied as u64) } #[view] @@ -236,13 +236,13 @@ module supra_framework::leader_ban_registry { } }); - vector::for_each(pool_addresses_to_remove, |p| { - let (is_exist, index) = vector::find(&mut ban_registry.bans, |v|{ v.pool_address == p }); + vector::for_each_ref(&pool_addresses_to_remove, |p| { + let (is_exist, index) = vector::find(&mut ban_registry.bans, |v|{ &v.pool_address == p }); if (is_exist) { vector::swap_remove(&mut ban_registry.bans, index); if (features::module_event_enabled()) { event::emit(BanRemoved { - pool_address: p, + pool_address: *p, epoch: latest_view.epoch, round: latest_view.round }); From aae7d9d328c3ed30bb9b8f5512039627fb5044a4 Mon Sep 17 00:00:00 2001 From: Dhaval Purohit Date: Fri, 3 Oct 2025 20:30:45 +0530 Subject: [PATCH 05/14] resolved comments --- .../configs/leader_ban_registry_config.move | 4 +- .../sources/leader_ban_registry.move | 146 ++-- .../supra-framework/sources/stake.move | 33 +- .../supra-stdlib/doc/bls12381_bulletproofs.md | 12 +- .../framework/supra-stdlib/doc/decode_bcs.md | 729 ++++++++++++++++++ .../framework/supra-stdlib/doc/overview.md | 1 + .../supra-stdlib/sources/decode_bcs.move | 2 +- 7 files changed, 847 insertions(+), 80 deletions(-) create mode 100644 aptos-move/framework/supra-stdlib/doc/decode_bcs.md diff --git a/aptos-move/framework/supra-framework/sources/configs/leader_ban_registry_config.move b/aptos-move/framework/supra-framework/sources/configs/leader_ban_registry_config.move index 4bad156af0572..e8bbc2c40c526 100644 --- a/aptos-move/framework/supra-framework/sources/configs/leader_ban_registry_config.move +++ b/aptos-move/framework/supra-framework/sources/configs/leader_ban_registry_config.move @@ -92,7 +92,7 @@ module supra_framework::leader_ban_registry_config { } #[view] - public fun get_ban_registry_param(): (vector, u8) acquires BanRegistryParameters { + public fun get_ban_registry_params(): (vector, u8) acquires BanRegistryParameters { if (!exists(@supra_framework)) { let ban_registry_config = borrow_global(@supra_framework); return ( @@ -104,7 +104,7 @@ module supra_framework::leader_ban_registry_config { } #[view] - public fun get_ban_registry_param_v0(): (u8, u32, u8) acquires BanRegistryParameters, BanRegistryParametersV0 { + public fun get_ban_registry_params_v0(): (u8, u32, u8) acquires BanRegistryParameters, BanRegistryParametersV0 { if (!exists(@supra_framework)) { let ban_registry_config = borrow_global(@supra_framework); if (ban_registry_config.version == 0) { diff --git a/aptos-move/framework/supra-framework/sources/leader_ban_registry.move b/aptos-move/framework/supra-framework/sources/leader_ban_registry.move index c7697c38c9a7a..15b6fe6e7c5d3 100644 --- a/aptos-move/framework/supra-framework/sources/leader_ban_registry.move +++ b/aptos-move/framework/supra-framework/sources/leader_ban_registry.move @@ -21,14 +21,14 @@ module supra_framework::leader_ban_registry { const ELATEST_VIEW_ALREADY_EXISTS: u64 = 3; /// Holds metrics for banned round, epoch and round server in prev epoch - struct ActiveBan has store, drop { + struct ActiveBan has store, drop, copy { epoch_earned: u64, // EPOCH round_earned: u64, // ROUND rounds_served_in_previous_epochs: u64 // ROUND } /// Holds validator metrics regarding duration pool address etc - struct ValidatorBansWithAddress has store, drop { + struct ValidatorBansWithAddress has store, drop, copy { active: ActiveBan, consecutive_bans: u32, pool_address: address @@ -46,14 +46,7 @@ module supra_framework::leader_ban_registry { } #[event] - struct BanAdded has drop, store { - pool_address: address, - epoch: u64, - round: u64 - } - - #[event] - struct BanUpdated has drop, store { + struct Bannned has drop, store { pool_address: address, epoch: u64, round: u64, @@ -61,7 +54,7 @@ module supra_framework::leader_ban_registry { } #[event] - struct BanRemoved has drop, store { + struct Reinstated has drop, store { pool_address: address, epoch: u64, round: u64 @@ -100,9 +93,9 @@ module supra_framework::leader_ban_registry { #[view] public fun get_max_ban_duration(): u64 { - let max_elections_denied = leader_ban_registry_config::get_max_elections_denied() as u64; + let max_elections_denied = leader_ban_registry_config::get_max_elections_denied(); let committee_size = stake::get_committee_size(); - committee_size * max_elections_denied + committee_size * (max_elections_denied as u64) } /// Add or update the ban registry as per block metadata @@ -123,17 +116,24 @@ module supra_framework::leader_ban_registry { latest_view.round = round_earned; // ban the failed proposer indices - ban(epoch_earned, round_earned, failed_proposer_indices, ban_registry); + bans(latest_view, failed_proposer_indices, ban_registry); + + // removing bans whose duration is over + reinstate_bans(latest_view, ban_registry); // unban the proposer if (std::option::is_some(&proposer_index)) { let extracted_proposer_index = *std::option::borrow(&mut proposer_index); - unban(epoch_earned, round_earned, extracted_proposer_index, ban_registry); + reinstate_proposer(latest_view, extracted_proposer_index, ban_registry); }; } /// Adds failed proposer indices to ban registry - fun ban(epoch_earned: u64, round_earned: u64, failed_proposer_indices: vector, ban_registry: &mut BanRegistry) + fun bans( + latest_view: &LatestView, + failed_proposer_indices: vector, + ban_registry: &mut BanRegistry, + ) { let initial_ban_duration = get_initial_ban_duration(); if (initial_ban_duration == 0) { @@ -144,27 +144,28 @@ module supra_framework::leader_ban_registry { let validator_pool_address_opt = stake::get_pool_address_from_index(failed_validator_index); if (option::is_some(&validator_pool_address_opt)) { let validator_pool_address = option::extract(&mut validator_pool_address_opt); - let (is_banned, index) = vector::find(&ban_registry.bans, |v| { + let (is_ban_exists, index) = vector::find(&ban_registry.bans, |v| { + let v: &ValidatorBansWithAddress = v; validator_pool_address == v.pool_address }); - if (is_banned) { + if (is_ban_exists) { let bans = vector::borrow_mut(&mut ban_registry.bans, index); - bans.consecutive_bans += 1; - if (duration(bans) >= get_max_ban_duration()) { + bans.consecutive_bans = bans.consecutive_bans + 1; + if (remaining_duration(bans, latest_view) == 0) { vector::swap_remove(&mut ban_registry.bans, index); if (features::module_event_enabled()) { - event::emit(BanRemoved { + event::emit(Reinstated { pool_address: validator_pool_address, - epoch: epoch_earned, - round: round_earned + epoch: latest_view.epoch, + round: latest_view.round }); } } else { if (features::module_event_enabled()) { - event::emit(BanUpdated { + event::emit(Bannned { pool_address: validator_pool_address, - epoch: epoch_earned, - round: round_earned, + epoch: latest_view.epoch, + round: latest_view.round, consecutive_bans: bans.consecutive_bans }); } @@ -172,21 +173,23 @@ module supra_framework::leader_ban_registry { } else { let ban_registry_len = vector::length(&ban_registry.bans); if (can_be_added_in_ban(ban_registry_len)) { - vector::push_back(&mut ban_registry.bans, ValidatorBansWithAddress { + let ban_with_address = ValidatorBansWithAddress { active: ActiveBan { - epoch_earned, - round_earned, + epoch_earned: latest_view.epoch, + round_earned: latest_view.round, rounds_served_in_previous_epochs: 0 }, consecutive_bans: 0, pool_address: validator_pool_address - }); + }; + vector::push_back(&mut ban_registry.bans, ban_with_address); if (features::module_event_enabled()) { - event::emit(BanAdded { - pool_address: validator_pool_address, - epoch: epoch_earned, - round: round_earned + event::emit(Bannned { + pool_address: ban_with_address.pool_address, + epoch: ban_with_address.active.epoch_earned, + round: ban_with_address.active.round_earned, + consecutive_bans: ban_with_address.consecutive_bans }); } } @@ -195,17 +198,51 @@ module supra_framework::leader_ban_registry { }); } + /// Removes bans for those validator which duration is over + fun reinstate_bans(latest_view: &LatestView, ban_registry: &mut BanRegistry) { + let pool_address_for_duration_over = vector::empty(); + vector::for_each_ref(&ban_registry.bans, |v| { + if (remaining_duration(v, latest_view) == 0 ) { + vector::push_back(&mut pool_address_for_duration_over, v.pool_address); + } + }); + vector::for_each_ref(&pool_address_for_duration_over, |p| { + let (exists, index) = vector::find(&ban_registry.bans, |v| { + let v : &ValidatorBansWithAddress = v; + &v.pool_address == p + }); + if (exists) { + vector::swap_remove(&mut ban_registry.bans, index); + if (features::module_event_enabled()) { + event::emit(Reinstated { + epoch: latest_view.epoch, + round: latest_view.round, + pool_address: *p + }) + } + } + }); + } + /// Remvoing an entry of validator from registry if found at proposer index - fun unban(epoch_earned: u64, round_earned: u64, proposer_index: u64, ban_registry: &mut BanRegistry) + fun reinstate_proposer(latest_view: &LatestView, proposer_index: u64, ban_registry: &mut BanRegistry) { let validator_pool_address_opt= stake::get_pool_address_from_index(proposer_index); if (option::is_some(&validator_pool_address_opt)) { let validator_pool_address = option::extract(&mut validator_pool_address_opt); let (is_banned, index) = vector::find(&ban_registry.bans, |v| { + let v: &ValidatorBansWithAddress = v; validator_pool_address == v.pool_address }); if (is_banned) { vector::swap_remove(&mut ban_registry.bans, index); + if (features::module_event_enabled()) { + event::emit(Reinstated { + round: latest_view.round, + epoch: latest_view.epoch, + pool_address: validator_pool_address + }) + } }; }; } @@ -224,11 +261,12 @@ module supra_framework::leader_ban_registry { let updated_pool_addresses = stake::get_committee_pool_addresses(); let pool_addresses_to_remove = vector::empty(); vector::for_each_mut(&mut ban_registry.bans, |v| { + let v: &mut ValidatorBansWithAddress = v; if (has_started(&v.active, latest_view)) { if (latest_view.epoch > v.active.epoch_earned) { - v.active.rounds_served_in_previous_epochs += latest_view.round; + v.active.rounds_served_in_previous_epochs = v.active.rounds_served_in_previous_epochs + latest_view.round; } else { - v.active.rounds_served_in_previous_epochs += latest_view.round - v.active.round_earned; + v.active.rounds_served_in_previous_epochs = v.active.rounds_served_in_previous_epochs + latest_view.round - v.active.round_earned; } }; if (vector::contains(&updated_pool_addresses, &v.pool_address)) { @@ -237,11 +275,14 @@ module supra_framework::leader_ban_registry { }); vector::for_each_ref(&pool_addresses_to_remove, |p| { - let (is_exist, index) = vector::find(&mut ban_registry.bans, |v|{ &v.pool_address == p }); + let (is_exist, index) = vector::find(&ban_registry.bans, |v|{ + let v : &ValidatorBansWithAddress = v; + &v.pool_address == p + }); if (is_exist) { vector::swap_remove(&mut ban_registry.bans, index); if (features::module_event_enabled()) { - event::emit(BanRemoved { + event::emit(Reinstated { pool_address: *p, epoch: latest_view.epoch, round: latest_view.round @@ -256,28 +297,25 @@ module supra_framework::leader_ban_registry { active_ban.epoch_earned <= latest_view.epoch && active_ban.round_earned < latest_view.round } - /// Calculate the duration - fun duration(ban: &ValidatorBansWithAddress): u64 { + /// Calculate the number of rounds remaining in the a given ban. + fun remaining_duration(ban: &ValidatorBansWithAddress, latest_view: &LatestView): u64 { let initial_ban_duration = get_initial_ban_duration(); let max_ban_duration = get_max_ban_duration(); - let duration = initial_ban_duration * pow(2, ban.consecutive_bans as u64); - if (duration >= ban.active.rounds_served_in_previous_epochs) { - duration = duration - ban.active.rounds_served_in_previous_epochs; + let duration = initial_ban_duration * pow(2, (ban.consecutive_bans as u64)); + let duration = min(duration, max_ban_duration); + let rounds_served = ban.active.rounds_served_in_previous_epochs + latest_view.round; + if (duration >= rounds_served) { + duration - rounds_served } else { - duration = 0; - }; - min(duration, max_ban_duration) + 0 + } } + /// Returns true until ban registry size + minimum proposers required count less than committee size fun can_be_added_in_ban(ban_registry_len: u64) : bool { - let minimum_unbanned_proposer = leader_ban_registry_config::get_minimum_unbanned_proposers() as u64; + let minimum_unbanned_proposers = leader_ban_registry_config::get_minimum_unbanned_proposers(); let committee_size = stake::get_committee_size(); - if (committee_size > ban_registry_len) { - if (committee_size - ban_registry_len > minimum_unbanned_proposer) { - return true; - }; - }; - false + committee_size >= ban_registry_len + (minimum_unbanned_proposers as u64) } /// Validates registry initialised if not aborted with `EBAN_REGISTRY_NOT_INITIALIZED` diff --git a/aptos-move/framework/supra-framework/sources/stake.move b/aptos-move/framework/supra-framework/sources/stake.move index fabe920e7bc64..cc61128ec18ed 100644 --- a/aptos-move/framework/supra-framework/sources/stake.move +++ b/aptos-move/framework/supra-framework/sources/stake.move @@ -516,25 +516,22 @@ module supra_framework::stake { // it can happen that some validator may added a leave request in between // this can change the order of indexes in active validator // so need to iterate through all until find pool address in active and pending_active - let return_addr = option::none(); - vector::for_each_ref(&validator_set.active_validators, |validator_info| { - if (index == validator_info.config.validator_index) { - return_addr = option::some(validator_info.addr); - break; - } + let (is_index_exist, i) = vector::find(&validator_set.active_validators, |v| { + let v: &ValidatorInfo = v; + v.config.validator_index == index }); - // as we are sure that either the index can be found in active or pending inactive or none - // returning early without looping through pending_inactive if found - if (option::is_some(&return_addr)) { - return return_addr; + if (is_index_exist) { + let v_info = vector::borrow(&validator_set.active_validators, i); + return option::some(v_info.addr); }; - vector::for_each_ref(&validator_set.pending_inactive, |validator_info| { - if (index == validator_info.config.validator_index) { - return_addr = option::some(validator_info.addr); - break; - } + let (is_index_exist, i) = vector::find(&validator_set.pending_inactive, |v| { + let v: &ValidatorInfo = v; + v.config.validator_index == index }); - return return_addr; + if (is_index_exist) { + let v_info = vector::borrow(&validator_set.pending_inactive, i); + return option::some(v_info.addr); + }; }; option::none() } @@ -551,7 +548,7 @@ module supra_framework::stake { commitee_size } - /// Returns committee size + /// Returns pool addresses of current committee including pending inactive public(friend) fun get_committee_pool_addresses() : vector
acquires ValidatorSet { @@ -559,9 +556,11 @@ module supra_framework::stake { if (exists(@supra_framework)) { let validator_set = borrow_global(@supra_framework); vector::for_each_ref(&validator_set.active_validators, |validator_info| { + let validator_info: &ValidatorInfo = validator_info; vector::push_back(&mut pool_addresses, validator_info.addr); }); vector::for_each_ref(&validator_set.pending_inactive, |validator_info| { + let validator_info: &ValidatorInfo = validator_info; vector::push_back(&mut pool_addresses, validator_info.addr); }); }; diff --git a/aptos-move/framework/supra-stdlib/doc/bls12381_bulletproofs.md b/aptos-move/framework/supra-stdlib/doc/bls12381_bulletproofs.md index a61b068016a7b..d2fdbbb83fe3b 100644 --- a/aptos-move/framework/supra-stdlib/doc/bls12381_bulletproofs.md +++ b/aptos-move/framework/supra-stdlib/doc/bls12381_bulletproofs.md @@ -58,22 +58,22 @@ Represents a zero-knowledge range proof that a value committed inside a Pedersen ## Constants - + -There was an error deserializing the range proof. +The native functions have not been rolled out yet. -
const E_DESERIALIZE_RANGE_PROOF: u64 = 1;
+
const E_NATIVE_FUN_NOT_AVAILABLE: u64 = 4;
 
- + -The native functions have not been rolled out yet. +There was an error deserializing the range proof. -
const E_NATIVE_FUN_NOT_AVAILABLE: u64 = 4;
+
const E_DESERIALIZE_RANGE_PROOF: u64 = 1;
 
diff --git a/aptos-move/framework/supra-stdlib/doc/decode_bcs.md b/aptos-move/framework/supra-stdlib/doc/decode_bcs.md new file mode 100644 index 0000000000000..42393b8d8d485 --- /dev/null +++ b/aptos-move/framework/supra-stdlib/doc/decode_bcs.md @@ -0,0 +1,729 @@ + + + +# Module `0x1::decode_bcs` + +This module implements BCS (de)serialization in Move. +Full specification can be found here: https://github.com/diem/bcs + + +- [Struct `BCS`](#0x1_decode_bcs_BCS) +- [Constants](#@Constants_0) +- [Function `to_bytes`](#0x1_decode_bcs_to_bytes) +- [Function `new`](#0x1_decode_bcs_new) +- [Function `into_remainder_bytes`](#0x1_decode_bcs_into_remainder_bytes) +- [Function `peel_bool`](#0x1_decode_bcs_peel_bool) +- [Function `peel_u8`](#0x1_decode_bcs_peel_u8) +- [Function `peel_u16`](#0x1_decode_bcs_peel_u16) +- [Function `peel_u32`](#0x1_decode_bcs_peel_u32) +- [Function `peel_u64`](#0x1_decode_bcs_peel_u64) +- [Function `peel_u128`](#0x1_decode_bcs_peel_u128) +- [Function `peel_u256`](#0x1_decode_bcs_peel_u256) +- [Function `peel_vec_length`](#0x1_decode_bcs_peel_vec_length) +- [Function `peel_vec_bool`](#0x1_decode_bcs_peel_vec_bool) +- [Function `peel_vec_u8`](#0x1_decode_bcs_peel_vec_u8) +- [Function `peel_vec_u16`](#0x1_decode_bcs_peel_vec_u16) +- [Function `peel_vec_u32`](#0x1_decode_bcs_peel_vec_u32) +- [Function `peel_vec_u64`](#0x1_decode_bcs_peel_vec_u64) +- [Function `peel_vec_u128`](#0x1_decode_bcs_peel_vec_u128) +- [Function `peel_vec_u256`](#0x1_decode_bcs_peel_vec_u256) +- [Function `peel_vec_vec_u8`](#0x1_decode_bcs_peel_vec_vec_u8) +- [Function `peel_vec_vec_vec_u8`](#0x1_decode_bcs_peel_vec_vec_vec_u8) +- [Specification](#@Specification_1) + + +
use 0x1::bcs;
+use 0x1::vector;
+
+ + + + + +## Struct `BCS` + +A helper struct that saves resources on operations. For better +vector performance, it stores reversed bytes of the BCS and +enables use of vector::pop_back. + + +
struct BCS has copy, drop, store
+
+ + + +
+Fields + + +
+
+bytes: vector<u8> +
+
+ +
+
+ + +
+ + + +## Constants + + + + +For when ULEB byte is out of range (or not found). + + +
const ELenOutOfRange: u64 = 2;
+
+ + + + + +For when the boolean value different than 0 or 1. + + +
const ENotBool: u64 = 1;
+
+ + + + + +For when bytes length is less than required for deserialization. + + +
const EOutOfRange: u64 = 0;
+
+ + + + + +## Function `to_bytes` + +Get BCS serialized bytes for any value. +Re-exports stdlib bcs::to_bytes. + + +
public fun to_bytes<T>(value: &T): vector<u8>
+
+ + + +
+Implementation + + +
public fun to_bytes<T>(value: &T): vector<u8> {
+    bcs::to_bytes(value)
+}
+
+ + + +
+ + + +## Function `new` + +Creates a new instance of BCS wrapper that holds inversed +bytes for better performance. + + +
public fun new(bytes: vector<u8>): decode_bcs::BCS
+
+ + + +
+Implementation + + +
public fun new(bytes: vector<u8>): BCS {
+    v::reverse(&mut bytes);
+    BCS { bytes }
+}
+
+ + + +
+ + + +## Function `into_remainder_bytes` + +Unpack the BCS struct returning the leftover bytes. +Useful for passing the data further after partial deserialization. + + +
public fun into_remainder_bytes(bcs: decode_bcs::BCS): vector<u8>
+
+ + + +
+Implementation + + +
public fun into_remainder_bytes(bcs: BCS): vector<u8> {
+    let BCS { bytes } = bcs;
+    v::reverse(&mut bytes);
+    bytes
+}
+
+ + + +
+ + + +## Function `peel_bool` + +Read a bool value from bcs-serialized bytes. + + +
public fun peel_bool(bcs: &mut decode_bcs::BCS): bool
+
+ + + +
+Implementation + + +
public fun peel_bool(bcs: &mut BCS): bool {
+    let value = peel_u8(bcs);
+    if (value == 0) {
+        false
+    } else if (value == 1) {
+        true
+    } else {
+        abort ENotBool
+    }
+}
+
+ + + +
+ + + +## Function `peel_u8` + +Read u8 value from bcs-serialized bytes. + + +
public fun peel_u8(bcs: &mut decode_bcs::BCS): u8
+
+ + + +
+Implementation + + +
public fun peel_u8(bcs: &mut BCS): u8 {
+    assert!(v::length(&bcs.bytes) >= 1, EOutOfRange);
+    v::pop_back(&mut bcs.bytes)
+}
+
+ + + +
+ + + +## Function `peel_u16` + +Read u16 value from bcs-serialized bytes. + + +
public fun peel_u16(bcs: &mut decode_bcs::BCS): u16
+
+ + + +
+Implementation + + +
public fun peel_u16(bcs: &mut BCS): u16 {
+    assert!(v::length(&bcs.bytes) >= 2, EOutOfRange);
+    let (value, i) = (0u16, 0u8);
+    while (i < 16) {
+        let byte = (v::pop_back(&mut bcs.bytes) as u16);
+        value = value | (byte << i);
+        i = i + 8;
+    };
+    value
+}
+
+ + + +
+ + + +## Function `peel_u32` + +Read u32 value from bcs-serialized bytes. + + +
public fun peel_u32(bcs: &mut decode_bcs::BCS): u32
+
+ + + +
+Implementation + + +
public fun peel_u32(bcs: &mut BCS): u32 {
+    assert!(v::length(&bcs.bytes) >= 4, EOutOfRange);
+    let (value, i) = (0u32, 0u8);
+    while (i < 32) {
+        let byte = (v::pop_back(&mut bcs.bytes) as u32);
+        value = value | (byte << i);
+        i = i + 8;
+    };
+    value
+}
+
+ + + +
+ + + +## Function `peel_u64` + +Read u64 value from bcs-serialized bytes. + + +
public fun peel_u64(bcs: &mut decode_bcs::BCS): u64
+
+ + + +
+Implementation + + +
public fun peel_u64(bcs: &mut BCS): u64 {
+    assert!(v::length(&bcs.bytes) >= 8, EOutOfRange);
+    let (value, i) = (0u64, 0u8);
+    while (i < 64) {
+        let byte = (v::pop_back(&mut bcs.bytes) as u64);
+        value = value | (byte << i);
+        i = i + 8;
+    };
+    value
+}
+
+ + + +
+ + + +## Function `peel_u128` + +Read u128 value from bcs-serialized bytes. + + +
public fun peel_u128(bcs: &mut decode_bcs::BCS): u128
+
+ + + +
+Implementation + + +
public fun peel_u128(bcs: &mut BCS): u128 {
+    assert!(v::length(&bcs.bytes) >= 16, EOutOfRange);
+
+    let (value, i) = (0u128, 0u8);
+    while (i < 128) {
+        let byte = (v::pop_back(&mut bcs.bytes) as u128);
+        value = value | (byte << i);
+        i = i + 8;
+    };
+
+    value
+}
+
+ + + +
+ + + +## Function `peel_u256` + +Read u256 value from bcs-serialized bytes. + + +
public fun peel_u256(bcs: &mut decode_bcs::BCS): u256
+
+ + + +
+Implementation + + +
public fun peel_u256(bcs: &mut BCS): u256 {
+    assert!(v::length(&bcs.bytes) >= 16, EOutOfRange);
+
+    let (value, i) = (0u256, 0u8);
+    while (i < 255) {
+        let byte = (v::pop_back(&mut bcs.bytes) as u256);
+        value = value | (byte << i);
+        i = i + 8;
+    };
+
+    value
+}
+
+ + + +
+ + + +## Function `peel_vec_length` + +Read ULEB bytes expecting a vector length. Result should +then be used to perform peel_* operation LEN times. + +In BCS vector length is implemented with ULEB128; +See more here: https://en.wikipedia.org/wiki/LEB128 + + +
public fun peel_vec_length(bcs: &mut decode_bcs::BCS): u64
+
+ + + +
+Implementation + + +
public fun peel_vec_length(bcs: &mut BCS): u64 {
+    let (total, shift, len) = (0u64, 0, 0);
+    while (true) {
+        assert!(len <= 4, ELenOutOfRange);
+        let byte = (v::pop_back(&mut bcs.bytes) as u64);
+        len = len + 1;
+        total = total | ((byte & 0x7f) << shift);
+        if ((byte & 0x80) == 0) {
+            break
+        };
+        shift = shift + 7;
+    };
+    total
+}
+
+ + + +
+ + + +## Function `peel_vec_bool` + +Peel a vector of bool from serialized bytes. + + +
public fun peel_vec_bool(bcs: &mut decode_bcs::BCS): vector<bool>
+
+ + + +
+Implementation + + +
public fun peel_vec_bool(bcs: &mut BCS): vector<bool> {
+    let (len, i, res) = (peel_vec_length(bcs), 0, vector[]);
+    while (i < len) {
+        v::push_back(&mut res, peel_bool(bcs));
+        i = i + 1;
+    };
+    res
+}
+
+ + + +
+ + + +## Function `peel_vec_u8` + +Peel a vector of u8 (eg string) from serialized bytes. + + +
public fun peel_vec_u8(bcs: &mut decode_bcs::BCS): vector<u8>
+
+ + + +
+Implementation + + +
public fun peel_vec_u8(bcs: &mut BCS): vector<u8> {
+    let (len, i, res) = (peel_vec_length(bcs), 0, vector[]);
+    while (i < len) {
+        v::push_back(&mut res, peel_u8(bcs));
+        i = i + 1;
+    };
+    res
+}
+
+ + + +
+ + + +## Function `peel_vec_u16` + +Peel a vector of u16 (eg string) from serialized bytes. + + +
public fun peel_vec_u16(bcs: &mut decode_bcs::BCS): vector<u16>
+
+ + + +
+Implementation + + +
public fun peel_vec_u16(bcs: &mut BCS): vector<u16> {
+    let (len, i, res) = (peel_vec_length(bcs), 0, vector[]);
+    while (i < len) {
+        v::push_back(&mut res, peel_u16(bcs));
+        i = i + 1;
+    };
+    res
+}
+
+ + + +
+ + + +## Function `peel_vec_u32` + +Peel a vector of u32 (eg string) from serialized bytes. + + +
public fun peel_vec_u32(bcs: &mut decode_bcs::BCS): vector<u32>
+
+ + + +
+Implementation + + +
public fun peel_vec_u32(bcs: &mut BCS): vector<u32> {
+    let (len, i, res) = (peel_vec_length(bcs), 0, vector[]);
+    while (i < len) {
+        v::push_back(&mut res, peel_u32(bcs));
+        i = i + 1;
+    };
+    res
+}
+
+ + + +
+ + + +## Function `peel_vec_u64` + +Peel a vector of u64 from serialized bytes. + + +
public fun peel_vec_u64(bcs: &mut decode_bcs::BCS): vector<u64>
+
+ + + +
+Implementation + + +
public fun peel_vec_u64(bcs: &mut BCS): vector<u64> {
+    let (len, i, res) = (peel_vec_length(bcs), 0, vector[]);
+    while (i < len) {
+        v::push_back(&mut res, peel_u64(bcs));
+        i = i + 1;
+    };
+    res
+}
+
+ + + +
+ + + +## Function `peel_vec_u128` + +Peel a vector of u128 from serialized bytes. + + +
public fun peel_vec_u128(bcs: &mut decode_bcs::BCS): vector<u128>
+
+ + + +
+Implementation + + +
public fun peel_vec_u128(bcs: &mut BCS): vector<u128> {
+    let (len, i, res) = (peel_vec_length(bcs), 0, vector[]);
+    while (i < len) {
+        v::push_back(&mut res, peel_u128(bcs));
+        i = i + 1;
+    };
+    res
+}
+
+ + + +
+ + + +## Function `peel_vec_u256` + +Peel a vector of u256 from serialized bytes. + + +
public fun peel_vec_u256(bcs: &mut decode_bcs::BCS): vector<u256>
+
+ + + +
+Implementation + + +
public fun peel_vec_u256(bcs: &mut BCS): vector<u256> {
+    let (len, i, res) = (peel_vec_length(bcs), 0, vector[]);
+    while (i < len) {
+        v::push_back(&mut res, peel_u256(bcs));
+        i = i + 1;
+    };
+    res
+}
+
+ + + +
+ + + +## Function `peel_vec_vec_u8` + +Peel a vector<vector<u8>> (eg vec of string) from serialized bytes. + + +
public fun peel_vec_vec_u8(bcs: &mut decode_bcs::BCS): vector<vector<u8>>
+
+ + + +
+Implementation + + +
public fun peel_vec_vec_u8(bcs: &mut BCS): vector<vector<u8>> {
+    let (len, i, res) = (peel_vec_length(bcs), 0, vector[]);
+    while (i < len) {
+        v::push_back(&mut res, peel_vec_u8(bcs));
+        i = i + 1;
+    };
+    res
+}
+
+ + + +
+ + + +## Function `peel_vec_vec_vec_u8` + +Peel a vector<vector<vector<u8>>> (eg vec of string) from serialized bytes. + + +
public fun peel_vec_vec_vec_u8(bcs: &mut decode_bcs::BCS): vector<vector<vector<u8>>>
+
+ + + +
+Implementation + + +
public fun peel_vec_vec_vec_u8(bcs: &mut BCS): vector<vector<vector<u8>>> {
+    let (len, i, res) = (peel_vec_length(bcs), 0, vector[]);
+    while (i < len) {
+        v::push_back(&mut res, peel_vec_vec_u8(bcs));
+        i = i + 1;
+    };
+    res
+}
+
+ + + +
+ + + +## Specification + + + +
pragma verify = false;
+
+ + +[move-book]: https://aptos.dev/move/book/SUMMARY diff --git a/aptos-move/framework/supra-stdlib/doc/overview.md b/aptos-move/framework/supra-stdlib/doc/overview.md index d4b37dd198d1a..b2527026641ac 100644 --- a/aptos-move/framework/supra-stdlib/doc/overview.md +++ b/aptos-move/framework/supra-stdlib/doc/overview.md @@ -15,6 +15,7 @@ This is the reference documentation of the Supra standard library extension. - [`0x1::bls12381_bulletproofs`](bls12381_bulletproofs.md#0x1_bls12381_bulletproofs) - [`0x1::bls12381_pedersen`](bls12381_pedersen.md#0x1_bls12381_pedersen) - [`0x1::bls12381_scalar`](bls12381_scalar.md#0x1_bls12381_scalar) +- [`0x1::decode_bcs`](decode_bcs.md#0x1_decode_bcs) - [`0x1::enumerable_map`](enumerable_map.md#0x1_enumerable_map) - [`0x1::eth_trie`](eth_trie.md#0x1_eth_trie) - [`0x1::rlp`](rlp.md#0x1_rlp) diff --git a/aptos-move/framework/supra-stdlib/sources/decode_bcs.move b/aptos-move/framework/supra-stdlib/sources/decode_bcs.move index 570a38780a42c..5bf4aaca7dfdf 100644 --- a/aptos-move/framework/supra-stdlib/sources/decode_bcs.move +++ b/aptos-move/framework/supra-stdlib/sources/decode_bcs.move @@ -114,7 +114,7 @@ module supra_std::decode_bcs { assert!(v::length(&bcs.bytes) >= 16, EOutOfRange); let (value, i) = (0u256, 0u8); - while (i < 256) { + while (i < 255) { let byte = (v::pop_back(&mut bcs.bytes) as u256); value = value | (byte << i); i = i + 8; From 9aea22258028fd68833d0e0d424128a504589fa7 Mon Sep 17 00:00:00 2001 From: Dhaval Purohit Date: Mon, 6 Oct 2025 12:24:38 +0530 Subject: [PATCH 06/14] Minor formatiing --- .../sources/configs/leader_ban_registry_config.move | 8 ++++---- .../supra-framework/sources/leader_ban_registry.move | 12 ++++++------ .../framework/supra-framework/sources/stake.move | 4 ++-- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/aptos-move/framework/supra-framework/sources/configs/leader_ban_registry_config.move b/aptos-move/framework/supra-framework/sources/configs/leader_ban_registry_config.move index e8bbc2c40c526..d52cf616ccefd 100644 --- a/aptos-move/framework/supra-framework/sources/configs/leader_ban_registry_config.move +++ b/aptos-move/framework/supra-framework/sources/configs/leader_ban_registry_config.move @@ -39,7 +39,7 @@ module supra_framework::leader_ban_registry_config { assert!(!exists(@supra_framework), error::already_exists(EALREADY_INITIALISED)); let v0 = deserialise_v0_params(config); if (option::is_none(&v0)) { - abort error::invalid_argument(EINVALID_VERSION_BYTES); + abort error::invalid_argument(EINVALID_VERSION_BYTES) }; // we always init with version 0 move_to(supra_framework, BanRegistryParameters { config, version: 0 }); @@ -125,7 +125,7 @@ module supra_framework::leader_ban_registry_config { let ban_registry_config = borrow_global(@supra_framework); if (ban_registry_config.version == 0) { let ban_registry_params = borrow_global(@supra_framework); - return ban_registry_params.initial_elections_denied; + return ban_registry_params.initial_elections_denied } }; 0 @@ -137,7 +137,7 @@ module supra_framework::leader_ban_registry_config { let ban_registry_config = borrow_global(@supra_framework); if (ban_registry_config.version == 0) { let ban_registry_params = borrow_global(@supra_framework); - return ban_registry_params.max_elections_denied; + return ban_registry_params.max_elections_denied } }; 0 @@ -149,7 +149,7 @@ module supra_framework::leader_ban_registry_config { let ban_registry_config = borrow_global(@supra_framework); if (ban_registry_config.version == 0) { let ban_registry_params = borrow_global(@supra_framework); - return ban_registry_params.minimum_unbanned_proposers; + return ban_registry_params.minimum_unbanned_proposers } }; 0 diff --git a/aptos-move/framework/supra-framework/sources/leader_ban_registry.move b/aptos-move/framework/supra-framework/sources/leader_ban_registry.move index 15b6fe6e7c5d3..aba4ef64dfa2e 100644 --- a/aptos-move/framework/supra-framework/sources/leader_ban_registry.move +++ b/aptos-move/framework/supra-framework/sources/leader_ban_registry.move @@ -78,7 +78,7 @@ module supra_framework::leader_ban_registry { #[view] public fun get_ban_registry() : vector acquires BanRegistry { if (!exists(@supra_framework)) { - return (vector::empty()); + return vector::empty() }; let ban_registry = borrow_global(@supra_framework); ban_registry.bans @@ -105,10 +105,10 @@ module supra_framework::leader_ban_registry { acquires BanRegistry, LatestView { if (!exists(@supra_framework)) { - return ; + return }; if (!exists(@supra_framework)) { - return ; + return }; let ban_registry = borrow_global_mut(@supra_framework); let latest_view = borrow_global_mut(@supra_framework); @@ -137,7 +137,7 @@ module supra_framework::leader_ban_registry { { let initial_ban_duration = get_initial_ban_duration(); if (initial_ban_duration == 0) { - return; + return }; vector::for_each(failed_proposer_indices, |failed_validator_index| { @@ -250,10 +250,10 @@ module supra_framework::leader_ban_registry { /// Update counts on every epoch public(friend) fun on_new_epoch() acquires BanRegistry,LatestView { if (!exists(@supra_framework)) { - return; + return }; if (!exists(@supra_framework)) { - return; + return }; let latest_view = borrow_global(@supra_framework); let ban_registry = borrow_global_mut(@supra_framework); diff --git a/aptos-move/framework/supra-framework/sources/stake.move b/aptos-move/framework/supra-framework/sources/stake.move index cc61128ec18ed..38787b6263c3c 100644 --- a/aptos-move/framework/supra-framework/sources/stake.move +++ b/aptos-move/framework/supra-framework/sources/stake.move @@ -522,7 +522,7 @@ module supra_framework::stake { }); if (is_index_exist) { let v_info = vector::borrow(&validator_set.active_validators, i); - return option::some(v_info.addr); + return option::some(v_info.addr) }; let (is_index_exist, i) = vector::find(&validator_set.pending_inactive, |v| { let v: &ValidatorInfo = v; @@ -530,7 +530,7 @@ module supra_framework::stake { }); if (is_index_exist) { let v_info = vector::borrow(&validator_set.pending_inactive, i); - return option::some(v_info.addr); + return option::some(v_info.addr) }; }; option::none() From 106653506665daaf7fa0947920eaee08b964d5b3 Mon Sep 17 00:00:00 2001 From: Dhaval Purohit Date: Mon, 6 Oct 2025 20:20:10 +0530 Subject: [PATCH 07/14] added config tests --- .../configs/leader_ban_registry_config.move | 54 ++++++++++++-- .../supra-framework/sources/genesis.move | 11 +++ .../sources/leader_ban_registry.move | 28 ++++++- .../sources/reconfiguration_with_dkg.move | 2 + .../tests/test_leader_ban_registry.move | 74 ++++++++++++++++++- 5 files changed, 160 insertions(+), 9 deletions(-) diff --git a/aptos-move/framework/supra-framework/sources/configs/leader_ban_registry_config.move b/aptos-move/framework/supra-framework/sources/configs/leader_ban_registry_config.move index d52cf616ccefd..847976769308a 100644 --- a/aptos-move/framework/supra-framework/sources/configs/leader_ban_registry_config.move +++ b/aptos-move/framework/supra-framework/sources/configs/leader_ban_registry_config.move @@ -7,10 +7,15 @@ module supra_framework::leader_ban_registry_config { use supra_std::decode_bcs; use supra_framework::config_buffer; use supra_framework::system_addresses; + #[test_only] + use std::signer; friend supra_framework::genesis; friend supra_framework::reconfiguration_with_dkg; + #[test_only] + friend supra_framework::test_leader_ban_registry_config; + /// The provided on chain config bytes are empty or invalid const EINVALID_CONFIG: u64 = 1; /// The provided on chain config version should be equal or greater than existing @@ -21,14 +26,21 @@ module supra_framework::leader_ban_registry_config { const EALREADY_INITIALISED: u64 = 4; + /// Holds ban registry parameters bytes and it's version struct BanRegistryParameters has drop, key, store { + /// Denotes config bcs bytes config: vector, + /// Denotes config version version: u8 } + /// Ban registry parameters v0 struct BanRegistryParametersV0 has drop, key, store { + /// Denotes initial election count denied initial_elections_denied: u8, + /// Denotes max election count denied max_elections_denied: u32, + /// Denotes minimum unbanned proposer count minimum_unbanned_proposers: u8 } @@ -93,7 +105,7 @@ module supra_framework::leader_ban_registry_config { #[view] public fun get_ban_registry_params(): (vector, u8) acquires BanRegistryParameters { - if (!exists(@supra_framework)) { + if (exists(@supra_framework)) { let ban_registry_config = borrow_global(@supra_framework); return ( ban_registry_config.config, @@ -105,7 +117,7 @@ module supra_framework::leader_ban_registry_config { #[view] public fun get_ban_registry_params_v0(): (u8, u32, u8) acquires BanRegistryParameters, BanRegistryParametersV0 { - if (!exists(@supra_framework)) { + if (exists(@supra_framework)) { let ban_registry_config = borrow_global(@supra_framework); if (ban_registry_config.version == 0) { let ban_registry_params = borrow_global(@supra_framework); @@ -121,7 +133,7 @@ module supra_framework::leader_ban_registry_config { /// Provide initial election denied value public fun get_initial_elections_denied(): u8 acquires BanRegistryParameters, BanRegistryParametersV0 { - if (!exists(@supra_framework)) { + if (exists(@supra_framework)) { let ban_registry_config = borrow_global(@supra_framework); if (ban_registry_config.version == 0) { let ban_registry_params = borrow_global(@supra_framework); @@ -133,7 +145,7 @@ module supra_framework::leader_ban_registry_config { /// Provide max election denied value public fun get_max_elections_denied(): u32 acquires BanRegistryParameters, BanRegistryParametersV0 { - if (!exists(@supra_framework)) { + if (exists(@supra_framework)) { let ban_registry_config = borrow_global(@supra_framework); if (ban_registry_config.version == 0) { let ban_registry_params = borrow_global(@supra_framework); @@ -145,7 +157,7 @@ module supra_framework::leader_ban_registry_config { /// Provide minimum unbanned proposers value public fun get_minimum_unbanned_proposers(): u8 acquires BanRegistryParameters, BanRegistryParametersV0 { - if (!exists(@supra_framework)) { + if (exists(@supra_framework)) { let ban_registry_config = borrow_global(@supra_framework); if (ban_registry_config.version == 0) { let ban_registry_params = borrow_global(@supra_framework); @@ -171,4 +183,36 @@ module supra_framework::leader_ban_registry_config { }; option::none() } + + #[test_only] + public fun get_test_ban_registry_params_v0(): BanRegistryParametersV0 { + BanRegistryParametersV0 { + minimum_unbanned_proposers: 2, + max_elections_denied: 5, + initial_elections_denied : 1 + } + } + + #[test_only] + public fun get_custom_ban_registry_params_v0( + initial_elections_denied: u8, + max_elections_denied: u32, + minimum_unbanned_proposers: u8 + ): BanRegistryParametersV0 { + BanRegistryParametersV0 { + initial_elections_denied, + max_elections_denied, + minimum_unbanned_proposers + } + } + + #[test_only] + public fun check_ban_registry_params_exist(sender: &signer): bool { + exists(signer::address_of(sender)) + } + + #[test_only] + public fun check_ban_registry_params_v0_exist(sender: &signer): bool { + exists(signer::address_of(sender)) + } } diff --git a/aptos-move/framework/supra-framework/sources/genesis.move b/aptos-move/framework/supra-framework/sources/genesis.move index 11232efbda9d2..1f56d77c45192 100644 --- a/aptos-move/framework/supra-framework/sources/genesis.move +++ b/aptos-move/framework/supra-framework/sources/genesis.move @@ -19,6 +19,8 @@ module supra_framework::genesis { use supra_framework::evm_genesis_config; use supra_framework::create_signer::create_signer; use supra_framework::gas_schedule; + use supra_framework::leader_ban_registry; + use supra_framework::leader_ban_registry_config; use supra_framework::multisig_account; use supra_framework::pbo_delegation_pool; use supra_framework::reconfiguration; @@ -269,6 +271,15 @@ module supra_framework::genesis { evm_genesis_config::initialize(supra_framework, evm_genesis_config); } + /// Initialize the leader ban config + fun initialize_leader_ban_registry_config( + supra_framework: &signer, + leader_ban_registry_config: vector, + ) { + leader_ban_registry_config::initialize(supra_framework, leader_ban_registry_config); + leader_ban_registry::initialize_leader_ban_registry(supra_framework); + } + fun create_accounts(supra_framework: &signer, accounts: vector) { let unique_accounts = vector::empty(); diff --git a/aptos-move/framework/supra-framework/sources/leader_ban_registry.move b/aptos-move/framework/supra-framework/sources/leader_ban_registry.move index aba4ef64dfa2e..4b7328dddc9c3 100644 --- a/aptos-move/framework/supra-framework/sources/leader_ban_registry.move +++ b/aptos-move/framework/supra-framework/sources/leader_ban_registry.move @@ -12,6 +12,7 @@ module supra_framework::leader_ban_registry { use supra_framework::leader_ban_registry_config; friend supra_framework::block; + friend supra_framework::genesis; /// Leader ban registry already initialized const EBAN_REGISTRY_ALREADY_EXISTS: u64 = 1; @@ -22,41 +23,59 @@ module supra_framework::leader_ban_registry { /// Holds metrics for banned round, epoch and round server in prev epoch struct ActiveBan has store, drop, copy { - epoch_earned: u64, // EPOCH - round_earned: u64, // ROUND - rounds_served_in_previous_epochs: u64 // ROUND + /// Epoch when validator first got banned + epoch_earned: u64, + /// Round when validator first got banned + round_earned: u64, + /// Round count incremented on every epoch change + rounds_served_in_previous_epochs: u64 } /// Holds validator metrics regarding duration pool address etc struct ValidatorBansWithAddress has store, drop, copy { + /// Holds active ban counts active: ActiveBan, + /// Consecutive ban count consecutive_bans: u32, + /// Validator's pool address pool_address: address } /// Holds ban registry struct BanRegistry has drop, store, key { + /// List of validator active bans with pool address bans: vector } /// Holds latest processed round and epoch struct LatestView has drop, store, key { + /// Epoch epoch: u64, + /// Round round: u64 } #[event] + /// Emits when validator receives a ban or consucutive ban occurred struct Bannned has drop, store { + /// Validator's pool address pool_address: address, + /// Epoch epoch: u64, + /// Round round: u64, + /// Consecutive bans count consecutive_bans: u32 } #[event] + /// Emits when validator ban lifted due to ban expiry struct Reinstated has drop, store { + /// Validator's pool address pool_address: address, + /// Epoch epoch: u64, + /// Round round: u64 } @@ -76,6 +95,7 @@ module supra_framework::leader_ban_registry { } #[view] + /// Returns list of validators active ban with it's pool address public fun get_ban_registry() : vector acquires BanRegistry { if (!exists(@supra_framework)) { return vector::empty() @@ -85,6 +105,7 @@ module supra_framework::leader_ban_registry { } #[view] + /// Return initial ban duration public fun get_initial_ban_duration(): u64 { let initial_elections_denied = leader_ban_registry_config::get_initial_elections_denied(); let committee_size = stake::get_committee_size(); @@ -92,6 +113,7 @@ module supra_framework::leader_ban_registry { } #[view] + /// Returns max ban duration public fun get_max_ban_duration(): u64 { let max_elections_denied = leader_ban_registry_config::get_max_elections_denied(); let committee_size = stake::get_committee_size(); diff --git a/aptos-move/framework/supra-framework/sources/reconfiguration_with_dkg.move b/aptos-move/framework/supra-framework/sources/reconfiguration_with_dkg.move index e84d4f2ccea1e..b1878e83bc93d 100644 --- a/aptos-move/framework/supra-framework/sources/reconfiguration_with_dkg.move +++ b/aptos-move/framework/supra-framework/sources/reconfiguration_with_dkg.move @@ -9,6 +9,7 @@ module supra_framework::reconfiguration_with_dkg { use supra_framework::jwk_consensus_config; use supra_framework::jwks; use supra_framework::keyless_account; + use supra_framework::leader_ban_registry_config; use supra_framework::randomness_api_v0_config; use supra_framework::randomness_config; use supra_framework::randomness_config_seqnum; @@ -58,6 +59,7 @@ module supra_framework::reconfiguration_with_dkg { jwk_consensus_config::on_new_epoch(framework); jwks::on_new_epoch(framework); keyless_account::on_new_epoch(framework); + leader_ban_registry_config::on_new_epoch(framework); randomness_config_seqnum::on_new_epoch(framework); randomness_config::on_new_epoch(framework); randomness_api_v0_config::on_new_epoch(framework); diff --git a/aptos-move/framework/supra-framework/tests/test_leader_ban_registry.move b/aptos-move/framework/supra-framework/tests/test_leader_ban_registry.move index b9b4853d1e5a7..34fe9cfb3ddf5 100644 --- a/aptos-move/framework/supra-framework/tests/test_leader_ban_registry.move +++ b/aptos-move/framework/supra-framework/tests/test_leader_ban_registry.move @@ -1,2 +1,74 @@ #[test_only] -module std::test_leader_ban_registry {} +module supra_framework::test_leader_ban_registry_config { + use std::bcs; + use std::vector; + use supra_framework::config_buffer; + use supra_framework::leader_ban_registry_config; + + #[test(sender = @0xdead)] + #[expected_failure(abort_code=0x50003, location=supra_framework::system_addresses)] // signer is not valid + fun test_signer(sender: &signer) { + let ban_registry_param = leader_ban_registry_config::get_test_ban_registry_params_v0(); + let config_bytes = bcs::to_bytes(&ban_registry_param); + leader_ban_registry_config::initialize(sender, config_bytes); + } + + #[test(sender = @supra_framework)] + #[expected_failure(abort_code=0x10001, location=supra_framework::leader_ban_registry_config)] // Invalid config bytes + fun test_empty_config_value(sender: &signer) { + let config_bytes = vector::empty(); + leader_ban_registry_config::initialize(sender, config_bytes); + } + + #[test(sender = @supra_framework)] + #[expected_failure(abort_code=0x10003, location=supra_framework::leader_ban_registry_config)] // Invalid version bytes + fun test_invalid_config_value(sender: &signer) { + let ban_registry_param = leader_ban_registry_config::get_test_ban_registry_params_v0(); + let config_bytes = bcs::to_bytes(&ban_registry_param); + vector::push_back(&mut config_bytes, 0); + leader_ban_registry_config::initialize(sender, config_bytes); + } + + public fun init_ban_registry_params(sender: &signer) { + let ban_registry_param = leader_ban_registry_config::get_test_ban_registry_params_v0(); + let config_bytes = bcs::to_bytes(&ban_registry_param); + leader_ban_registry_config::initialize(sender, config_bytes); + } + + #[test(sender = @supra_framework)] + public fun test_init_config_value(sender: &signer) { + init_ban_registry_params(sender); + assert!(leader_ban_registry_config::check_ban_registry_params_exist(sender), 1); + assert!(leader_ban_registry_config::check_ban_registry_params_v0_exist(sender), 2); + } + + #[test(sender = @supra_framework)] + #[expected_failure(abort_code=0x80004, location=supra_framework::leader_ban_registry_config)] // Already initialised + fun test_init_config_value_reinit(sender: &signer) { + init_ban_registry_params(sender); + init_ban_registry_params(sender); + } + + #[test(sender = @supra_framework)] + fun test_on_epoch_change(sender: &signer) { + config_buffer::initialize(sender); + assert!(!leader_ban_registry_config::check_ban_registry_params_exist(sender), 1); + let ban_registry_param = leader_ban_registry_config::get_test_ban_registry_params_v0(); + let config_bytes = bcs::to_bytes(&ban_registry_param); + leader_ban_registry_config::set_for_next_epoch(sender, config_bytes, 0); + leader_ban_registry_config::on_new_epoch(sender); + assert!(leader_ban_registry_config::check_ban_registry_params_exist(sender), 1); + assert!(leader_ban_registry_config::check_ban_registry_params_v0_exist(sender), 2); + + let ban_registry_param = leader_ban_registry_config::get_custom_ban_registry_params_v0( + 3, + 8, + 6); + let config_bytes = bcs::to_bytes(&ban_registry_param); + leader_ban_registry_config::set_for_next_epoch(sender, config_bytes, 0); + leader_ban_registry_config::on_new_epoch(sender); + assert!(leader_ban_registry_config::get_initial_elections_denied()==3, 3); + assert!(leader_ban_registry_config::get_max_elections_denied()==8, 4); + assert!(leader_ban_registry_config::get_minimum_unbanned_proposers()==6, 5); + } +} From 4872aa30c04817ef551311f10a94b3e0b995ba53 Mon Sep 17 00:00:00 2001 From: Dhaval Purohit Date: Fri, 10 Oct 2025 20:05:27 +0530 Subject: [PATCH 08/14] resolved comments --- ...der_ban_registry.move => test_leader_ban_registry_config.move} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename aptos-move/framework/supra-framework/tests/{test_leader_ban_registry.move => test_leader_ban_registry_config.move} (100%) diff --git a/aptos-move/framework/supra-framework/tests/test_leader_ban_registry.move b/aptos-move/framework/supra-framework/tests/test_leader_ban_registry_config.move similarity index 100% rename from aptos-move/framework/supra-framework/tests/test_leader_ban_registry.move rename to aptos-move/framework/supra-framework/tests/test_leader_ban_registry_config.move From 76b4a4539b28e286633a9a422956e0ed30a5475c Mon Sep 17 00:00:00 2001 From: Dhaval Purohit Date: Fri, 17 Oct 2025 19:28:33 +0530 Subject: [PATCH 09/14] resolved comments --- .../configs/leader_ban_registry_config.move | 7 +- .../sources/leader_ban_registry.move | 98 +++++++++++-------- .../sources/reconfiguration.move | 2 + .../supra-framework/sources/stake.move | 3 + .../tests/test_leader_ban_registry.move | 95 ++++++++++++++++++ .../test_leader_ban_registry_config.move | 16 +-- 6 files changed, 171 insertions(+), 50 deletions(-) create mode 100644 aptos-move/framework/supra-framework/tests/test_leader_ban_registry.move diff --git a/aptos-move/framework/supra-framework/sources/configs/leader_ban_registry_config.move b/aptos-move/framework/supra-framework/sources/configs/leader_ban_registry_config.move index 847976769308a..8c4bfcca81ff5 100644 --- a/aptos-move/framework/supra-framework/sources/configs/leader_ban_registry_config.move +++ b/aptos-move/framework/supra-framework/sources/configs/leader_ban_registry_config.move @@ -15,6 +15,8 @@ module supra_framework::leader_ban_registry_config { #[test_only] friend supra_framework::test_leader_ban_registry_config; + #[test_only] + friend supra_framework::test_leader_ban_registry; /// The provided on chain config bytes are empty or invalid const EINVALID_CONFIG: u64 = 1; @@ -57,7 +59,6 @@ module supra_framework::leader_ban_registry_config { move_to(supra_framework, BanRegistryParameters { config, version: 0 }); let v0_params = option::extract(&mut v0); move_to(supra_framework, v0_params); - } /// This can be called by on-chain governance to update on-chain configs for the next epoch. @@ -187,9 +188,9 @@ module supra_framework::leader_ban_registry_config { #[test_only] public fun get_test_ban_registry_params_v0(): BanRegistryParametersV0 { BanRegistryParametersV0 { - minimum_unbanned_proposers: 2, + initial_elections_denied: 1, max_elections_denied: 5, - initial_elections_denied : 1 + minimum_unbanned_proposers: 2 } } diff --git a/aptos-move/framework/supra-framework/sources/leader_ban_registry.move b/aptos-move/framework/supra-framework/sources/leader_ban_registry.move index 4b7328dddc9c3..06e726a1a1163 100644 --- a/aptos-move/framework/supra-framework/sources/leader_ban_registry.move +++ b/aptos-move/framework/supra-framework/sources/leader_ban_registry.move @@ -13,6 +13,10 @@ module supra_framework::leader_ban_registry { friend supra_framework::block; friend supra_framework::genesis; + friend supra_framework::reconfiguration; + + #[test_only] + friend supra_framework::test_leader_ban_registry; /// Leader ban registry already initialized const EBAN_REGISTRY_ALREADY_EXISTS: u64 = 1; @@ -48,7 +52,7 @@ module supra_framework::leader_ban_registry { } /// Holds latest processed round and epoch - struct LatestView has drop, store, key { + struct LatestView has drop, store, key, copy { /// Epoch epoch: u64, /// Round @@ -104,6 +108,20 @@ module supra_framework::leader_ban_registry { ban_registry.bans } + #[view] + /// Return initial ban duration + public fun get_latest_view(): LatestView + acquires LatestView { + if (!exists(@supra_framework)) { + return LatestView { + epoch: 0, + round: 0 + } + }; + let latest_view = borrow_global(@supra_framework); + *latest_view + } + #[view] /// Return initial ban duration public fun get_initial_ban_duration(): u64 { @@ -137,11 +155,11 @@ module supra_framework::leader_ban_registry { latest_view.epoch = epoch_earned; latest_view.round = round_earned; - // ban the failed proposer indices - bans(latest_view, failed_proposer_indices, ban_registry); + // ban the failed proposer + ban_failed_proposers(latest_view, failed_proposer_indices, ban_registry); - // removing bans whose duration is over - reinstate_bans(latest_view, ban_registry); + // remove expired bans + reinstate_expired_bans(latest_view, ban_registry); // unban the proposer if (std::option::is_some(&proposer_index)) { @@ -151,7 +169,7 @@ module supra_framework::leader_ban_registry { } /// Adds failed proposer indices to ban registry - fun bans( + fun ban_failed_proposers( latest_view: &LatestView, failed_proposer_indices: vector, ban_registry: &mut BanRegistry, @@ -166,35 +184,24 @@ module supra_framework::leader_ban_registry { let validator_pool_address_opt = stake::get_pool_address_from_index(failed_validator_index); if (option::is_some(&validator_pool_address_opt)) { let validator_pool_address = option::extract(&mut validator_pool_address_opt); - let (is_ban_exists, index) = vector::find(&ban_registry.bans, |v| { + let (is_banned, index) = vector::find(&ban_registry.bans, |v| { let v: &ValidatorBansWithAddress = v; validator_pool_address == v.pool_address }); - if (is_ban_exists) { + if (is_banned) { let bans = vector::borrow_mut(&mut ban_registry.bans, index); bans.consecutive_bans = bans.consecutive_bans + 1; - if (remaining_duration(bans, latest_view) == 0) { - vector::swap_remove(&mut ban_registry.bans, index); - if (features::module_event_enabled()) { - event::emit(Reinstated { - pool_address: validator_pool_address, - epoch: latest_view.epoch, - round: latest_view.round - }); - } - } else { - if (features::module_event_enabled()) { - event::emit(Bannned { - pool_address: validator_pool_address, - epoch: latest_view.epoch, - round: latest_view.round, - consecutive_bans: bans.consecutive_bans - }); - } + if (features::module_event_enabled()) { + event::emit(Bannned { + pool_address: validator_pool_address, + epoch: latest_view.epoch, + round: latest_view.round, + consecutive_bans: bans.consecutive_bans + }); } } else { let ban_registry_len = vector::length(&ban_registry.bans); - if (can_be_added_in_ban(ban_registry_len)) { + if (can_be_banned(ban_registry_len)) { let ban_with_address = ValidatorBansWithAddress { active: ActiveBan { epoch_earned: latest_view.epoch, @@ -221,19 +228,19 @@ module supra_framework::leader_ban_registry { } /// Removes bans for those validator which duration is over - fun reinstate_bans(latest_view: &LatestView, ban_registry: &mut BanRegistry) { + fun reinstate_expired_bans(latest_view: &LatestView, ban_registry: &mut BanRegistry) { let pool_address_for_duration_over = vector::empty(); vector::for_each_ref(&ban_registry.bans, |v| { - if (remaining_duration(v, latest_view) == 0 ) { + if (remaining_duration(v, latest_view) == 0) { vector::push_back(&mut pool_address_for_duration_over, v.pool_address); } }); vector::for_each_ref(&pool_address_for_duration_over, |p| { - let (exists, index) = vector::find(&ban_registry.bans, |v| { + let (is_banned, index) = vector::find(&ban_registry.bans, |v| { let v : &ValidatorBansWithAddress = v; &v.pool_address == p }); - if (exists) { + if (is_banned) { vector::swap_remove(&mut ban_registry.bans, index); if (features::module_event_enabled()) { event::emit(Reinstated { @@ -249,7 +256,7 @@ module supra_framework::leader_ban_registry { /// Remvoing an entry of validator from registry if found at proposer index fun reinstate_proposer(latest_view: &LatestView, proposer_index: u64, ban_registry: &mut BanRegistry) { - let validator_pool_address_opt= stake::get_pool_address_from_index(proposer_index); + let validator_pool_address_opt = stake::get_pool_address_from_index(proposer_index); if (option::is_some(&validator_pool_address_opt)) { let validator_pool_address = option::extract(&mut validator_pool_address_opt); let (is_banned, index) = vector::find(&ban_registry.bans, |v| { @@ -270,6 +277,7 @@ module supra_framework::leader_ban_registry { } /// Update counts on every epoch + /// Only run after committee has been updated from reconfigure public(friend) fun on_new_epoch() acquires BanRegistry,LatestView { if (!exists(@supra_framework)) { return @@ -280,8 +288,10 @@ module supra_framework::leader_ban_registry { let latest_view = borrow_global(@supra_framework); let ban_registry = borrow_global_mut(@supra_framework); - let updated_pool_addresses = stake::get_committee_pool_addresses(); - let pool_addresses_to_remove = vector::empty(); + // The pool addresses of the validators for the new epoch. + let new_committee_pool_addresses = stake::get_committee_pool_addresses(); + // The pool addresses of the validators that have left the committee. + let retired_validators = vector::empty(); vector::for_each_mut(&mut ban_registry.bans, |v| { let v: &mut ValidatorBansWithAddress = v; if (has_started(&v.active, latest_view)) { @@ -291,17 +301,17 @@ module supra_framework::leader_ban_registry { v.active.rounds_served_in_previous_epochs = v.active.rounds_served_in_previous_epochs + latest_view.round - v.active.round_earned; } }; - if (vector::contains(&updated_pool_addresses, &v.pool_address)) { - vector::push_back(&mut pool_addresses_to_remove, v.pool_address) + if (!vector::contains(&new_committee_pool_addresses, &v.pool_address)) { + vector::push_back(&mut retired_validators, v.pool_address) } }); - vector::for_each_ref(&pool_addresses_to_remove, |p| { - let (is_exist, index) = vector::find(&ban_registry.bans, |v|{ + vector::for_each_ref(&retired_validators, |p| { + let (is_banned, index) = vector::find(&ban_registry.bans, |v|{ let v : &ValidatorBansWithAddress = v; &v.pool_address == p }); - if (is_exist) { + if (is_banned) { vector::swap_remove(&mut ban_registry.bans, index); if (features::module_event_enabled()) { event::emit(Reinstated { @@ -316,7 +326,8 @@ module supra_framework::leader_ban_registry { /// Checks if ban has already started fun has_started(active_ban: &ActiveBan, latest_view: &LatestView): bool { - active_ban.epoch_earned <= latest_view.epoch && active_ban.round_earned < latest_view.round + active_ban.epoch_earned < latest_view.epoch || + (active_ban.epoch_earned == latest_view.epoch && active_ban.round_earned < latest_view.round) } /// Calculate the number of rounds remaining in the a given ban. @@ -334,7 +345,7 @@ module supra_framework::leader_ban_registry { } /// Returns true until ban registry size + minimum proposers required count less than committee size - fun can_be_added_in_ban(ban_registry_len: u64) : bool { + fun can_be_banned(ban_registry_len: u64) : bool { let minimum_unbanned_proposers = leader_ban_registry_config::get_minimum_unbanned_proposers(); let committee_size = stake::get_committee_size(); committee_size >= ban_registry_len + (minimum_unbanned_proposers as u64) @@ -347,4 +358,9 @@ module supra_framework::leader_ban_registry { error::invalid_state(EBAN_REGISTRY_NOT_INITIALIZED) ); } + + #[test_only] + public fun get_pool_address_from_vp(validator_with_pool_addr: &ValidatorBansWithAddress) : address { + validator_with_pool_addr.pool_address + } } diff --git a/aptos-move/framework/supra-framework/sources/reconfiguration.move b/aptos-move/framework/supra-framework/sources/reconfiguration.move index 5e77f418da602..ee870c47b1961 100644 --- a/aptos-move/framework/supra-framework/sources/reconfiguration.move +++ b/aptos-move/framework/supra-framework/sources/reconfiguration.move @@ -4,6 +4,7 @@ module supra_framework::reconfiguration { use std::error; use std::features; use std::signer; + use supra_framework::leader_ban_registry; use supra_framework::account; use supra_framework::automation_registry; @@ -152,6 +153,7 @@ module supra_framework::reconfiguration { // Call stake to compute the new validator set and distribute rewards and transaction fees. stake::on_new_epoch(); + leader_ban_registry::on_new_epoch(); storage_gas::on_reconfig(); assert!(current_time > config_ref.last_reconfiguration_time, error::invalid_state(EINVALID_BLOCK_TIME)); diff --git a/aptos-move/framework/supra-framework/sources/stake.move b/aptos-move/framework/supra-framework/sources/stake.move index 38787b6263c3c..5717ce4b2703f 100644 --- a/aptos-move/framework/supra-framework/sources/stake.move +++ b/aptos-move/framework/supra-framework/sources/stake.move @@ -42,6 +42,9 @@ module supra_framework::stake { friend supra_framework::transaction_fee; friend supra_framework::leader_ban_registry; + #[test_only] + friend supra_framework::test_leader_ban_registry; + /// Validator Config not published. const EVALIDATOR_CONFIG: u64 = 1; /// Not enough stake to join validator set. diff --git a/aptos-move/framework/supra-framework/tests/test_leader_ban_registry.move b/aptos-move/framework/supra-framework/tests/test_leader_ban_registry.move new file mode 100644 index 0000000000000..de78c3479526e --- /dev/null +++ b/aptos-move/framework/supra-framework/tests/test_leader_ban_registry.move @@ -0,0 +1,95 @@ +#[test_only] +module std::test_leader_ban_registry { + use std::bcs; + use std::option; + use std::vector; + use aptos_std::ed25519; + use supra_framework::supra_coin; + use supra_framework::coin; + use supra_framework::supra_coin::SupraCoin; + use supra_framework::stake; + use supra_framework::leader_ban_registry; + use supra_framework::leader_ban_registry_config; + use std::signer; + use supra_framework::account; + + #[test(sender = @supra_framework, validator_1 = @0xdead01, validator_2 = @0xdead02, validator_3 = @0xdead03, validator_4 = @0xdead04)] + fun test_ban_registy_e2e( + sender: &signer, + validator_1: &signer, + validator_2: &signer, + validator_3: &signer, + validator_4: &signer + ) { + // initialise ban registry config + let ban_params = leader_ban_registry_config::get_test_ban_registry_params_v0(); + let ban_config_bytes = bcs::to_bytes(&ban_params); + leader_ban_registry_config::initialize(sender,ban_config_bytes); + + // initialise ban registry + leader_ban_registry::initialize_leader_ban_registry(sender); + + // initialise stake module + let (_v_1_s_key, v_1_p_key) = ed25519::generate_keys(); + let (_v_2_s_key, v_2_p_key) = ed25519::generate_keys(); + let (_v_3_s_key, v_3_p_key) = ed25519::generate_keys(); + let (_v_4_s_key, v_4_p_key) = ed25519::generate_keys(); + + stake::initialize_for_test(sender); + account::create_account_for_test(signer::address_of(validator_1)); + coin::register(validator_1); + supra_coin::mint(sender, signer::address_of(validator_1), 10000000000); + + account::create_account_for_test(signer::address_of(validator_2)); + coin::register(validator_2); + supra_coin::mint(sender, signer::address_of(validator_2), 10000000000); + + account::create_account_for_test(signer::address_of(validator_3)); + coin::register(validator_3); + supra_coin::mint(sender, signer::address_of(validator_3), 10000000000); + + account::create_account_for_test(signer::address_of(validator_4)); + coin::register(validator_4); + supra_coin::mint(sender, signer::address_of(validator_4), 10000000000); + + let active_coin = coin::withdraw(validator_1, 500); + let pending_coin = coin::withdraw(validator_1, 1000); + stake::create_stake_pool(validator_1, active_coin, pending_coin, 500000000); + + let active_coin = coin::withdraw(validator_2, 500); + let pending_coin = coin::withdraw(validator_2, 1000); + stake::create_stake_pool(validator_2, active_coin, pending_coin, 500000000); + + let active_coin = coin::withdraw(validator_3, 500); + let pending_coin = coin::withdraw(validator_3, 1000); + stake::create_stake_pool(validator_3, active_coin, pending_coin, 500000000); + + let active_coin = coin::withdraw(validator_4, 500); + let pending_coin = coin::withdraw(validator_4, 1000); + stake::create_stake_pool(validator_4, active_coin, pending_coin, 500000000); + + stake::join_validator_set_for_test(&ed25519::public_key_to_unvalidated(&v_4_p_key), validator_4, signer::address_of(validator_4), false); + stake::join_validator_set_for_test(&ed25519::public_key_to_unvalidated(&v_3_p_key), validator_3,signer::address_of(validator_3), false); + stake::join_validator_set_for_test(&ed25519::public_key_to_unvalidated(&v_2_p_key), validator_2, signer::address_of(validator_2), false); + stake::join_validator_set_for_test(&ed25519::public_key_to_unvalidated(&v_1_p_key), validator_1, signer::address_of(validator_1), true); // on epoch change + + // update ban registry + leader_ban_registry::update_ban_registry(0, 0, option::some(0), vector::empty()); + + let ban_registry = leader_ban_registry::get_ban_registry(); + assert!(vector::length(&ban_registry) == 0, 1); + + leader_ban_registry::update_ban_registry(0, 1, option::some(2), vector[1]); + let ban_registry = leader_ban_registry::get_ban_registry(); + assert!(vector::length(&ban_registry) == 1, 1); + + aptos_std::debug::print(&ban_registry); + aptos_std::debug::print(&stake::get_committee_pool_addresses()); + + assert!(signer::address_of(validator_2) == leader_ban_registry::get_pool_address_from_vp(vector::borrow(&ban_registry, 0)), 2 ); + + leader_ban_registry::update_ban_registry(0, 8, option::some(2), vector[1]); + let ban_registry = leader_ban_registry::get_ban_registry(); + aptos_std::debug::print(&ban_registry); + } +} diff --git a/aptos-move/framework/supra-framework/tests/test_leader_ban_registry_config.move b/aptos-move/framework/supra-framework/tests/test_leader_ban_registry_config.move index 34fe9cfb3ddf5..6227af230ad13 100644 --- a/aptos-move/framework/supra-framework/tests/test_leader_ban_registry_config.move +++ b/aptos-move/framework/supra-framework/tests/test_leader_ban_registry_config.move @@ -60,15 +60,19 @@ module supra_framework::test_leader_ban_registry_config { assert!(leader_ban_registry_config::check_ban_registry_params_exist(sender), 1); assert!(leader_ban_registry_config::check_ban_registry_params_v0_exist(sender), 2); + let updated_initial_e_denied = 3; + let updated_max_e_denied = 8; + let updated_minimum_u_proposers = 6; + let ban_registry_param = leader_ban_registry_config::get_custom_ban_registry_params_v0( - 3, - 8, - 6); + updated_initial_e_denied, + updated_max_e_denied, + updated_minimum_u_proposers); let config_bytes = bcs::to_bytes(&ban_registry_param); leader_ban_registry_config::set_for_next_epoch(sender, config_bytes, 0); leader_ban_registry_config::on_new_epoch(sender); - assert!(leader_ban_registry_config::get_initial_elections_denied()==3, 3); - assert!(leader_ban_registry_config::get_max_elections_denied()==8, 4); - assert!(leader_ban_registry_config::get_minimum_unbanned_proposers()==6, 5); + assert!(leader_ban_registry_config::get_initial_elections_denied()==updated_initial_e_denied, 3); + assert!(leader_ban_registry_config::get_max_elections_denied()==updated_max_e_denied, 4); + assert!(leader_ban_registry_config::get_minimum_unbanned_proposers()==updated_minimum_u_proposers, 5); } } From db6d394dcadc033682cdeff787d12cff5bd271de Mon Sep 17 00:00:00 2001 From: Dhaval Purohit Date: Thu, 23 Oct 2025 19:33:21 +0530 Subject: [PATCH 10/14] added few more cases --- .../sources/leader_ban_registry.move | 13 ++- .../tests/test_leader_ban_registry.move | 101 +++++++++++++++--- 2 files changed, 95 insertions(+), 19 deletions(-) diff --git a/aptos-move/framework/supra-framework/sources/leader_ban_registry.move b/aptos-move/framework/supra-framework/sources/leader_ban_registry.move index 06e726a1a1163..2830a2914a962 100644 --- a/aptos-move/framework/supra-framework/sources/leader_ban_registry.move +++ b/aptos-move/framework/supra-framework/sources/leader_ban_registry.move @@ -336,7 +336,11 @@ module supra_framework::leader_ban_registry { let max_ban_duration = get_max_ban_duration(); let duration = initial_ban_duration * pow(2, (ban.consecutive_bans as u64)); let duration = min(duration, max_ban_duration); - let rounds_served = ban.active.rounds_served_in_previous_epochs + latest_view.round; + let rounds_served = if (latest_view.epoch > ban.active.epoch_earned) { + ban.active.rounds_served_in_previous_epochs + latest_view.round + } else { + latest_view.round - ban.active.round_earned + }; if (duration >= rounds_served) { duration - rounds_served } else { @@ -348,7 +352,7 @@ module supra_framework::leader_ban_registry { fun can_be_banned(ban_registry_len: u64) : bool { let minimum_unbanned_proposers = leader_ban_registry_config::get_minimum_unbanned_proposers(); let committee_size = stake::get_committee_size(); - committee_size >= ban_registry_len + (minimum_unbanned_proposers as u64) + committee_size > ban_registry_len + (minimum_unbanned_proposers as u64) } /// Validates registry initialised if not aborted with `EBAN_REGISTRY_NOT_INITIALIZED` @@ -363,4 +367,9 @@ module supra_framework::leader_ban_registry { public fun get_pool_address_from_vp(validator_with_pool_addr: &ValidatorBansWithAddress) : address { validator_with_pool_addr.pool_address } + + #[test_only] + public fun get_consecutive_count_from_vp(validator_with_pool_addr: &ValidatorBansWithAddress) : u32 { + validator_with_pool_addr.consecutive_bans + } } diff --git a/aptos-move/framework/supra-framework/tests/test_leader_ban_registry.move b/aptos-move/framework/supra-framework/tests/test_leader_ban_registry.move index de78c3479526e..50bfc7d8f1a92 100644 --- a/aptos-move/framework/supra-framework/tests/test_leader_ban_registry.move +++ b/aptos-move/framework/supra-framework/tests/test_leader_ban_registry.move @@ -13,22 +13,13 @@ module std::test_leader_ban_registry { use std::signer; use supra_framework::account; - #[test(sender = @supra_framework, validator_1 = @0xdead01, validator_2 = @0xdead02, validator_3 = @0xdead03, validator_4 = @0xdead04)] - fun test_ban_registy_e2e( + fun setup_staking_modules( sender: &signer, validator_1: &signer, validator_2: &signer, validator_3: &signer, validator_4: &signer ) { - // initialise ban registry config - let ban_params = leader_ban_registry_config::get_test_ban_registry_params_v0(); - let ban_config_bytes = bcs::to_bytes(&ban_params); - leader_ban_registry_config::initialize(sender,ban_config_bytes); - - // initialise ban registry - leader_ban_registry::initialize_leader_ban_registry(sender); - // initialise stake module let (_v_1_s_key, v_1_p_key) = ed25519::generate_keys(); let (_v_2_s_key, v_2_p_key) = ed25519::generate_keys(); @@ -72,24 +63,100 @@ module std::test_leader_ban_registry { stake::join_validator_set_for_test(&ed25519::public_key_to_unvalidated(&v_3_p_key), validator_3,signer::address_of(validator_3), false); stake::join_validator_set_for_test(&ed25519::public_key_to_unvalidated(&v_2_p_key), validator_2, signer::address_of(validator_2), false); stake::join_validator_set_for_test(&ed25519::public_key_to_unvalidated(&v_1_p_key), validator_1, signer::address_of(validator_1), true); // on epoch change + } + + #[test(sender = @supra_framework, validator_1 = @0xdead01, validator_2 = @0xdead02, validator_3 = @0xdead03, validator_4 = @0xdead04)] + fun test_ban_registy_e2e( + sender: &signer, + validator_1: &signer, + validator_2: &signer, + validator_3: &signer, + validator_4: &signer + ) { + // initialise ban registry config + let ban_params = leader_ban_registry_config::get_test_ban_registry_params_v0(); + let ban_config_bytes = bcs::to_bytes(&ban_params); + leader_ban_registry_config::initialize(sender,ban_config_bytes); + + // initialise ban registry + leader_ban_registry::initialize_leader_ban_registry(sender); + setup_staking_modules(sender, validator_1, validator_2, validator_3, validator_4); // update ban registry leader_ban_registry::update_ban_registry(0, 0, option::some(0), vector::empty()); - let ban_registry = leader_ban_registry::get_ban_registry(); assert!(vector::length(&ban_registry) == 0, 1); + // Let's say for round 1 validator 2 failed to propose leader_ban_registry::update_ban_registry(0, 1, option::some(2), vector[1]); let ban_registry = leader_ban_registry::get_ban_registry(); - assert!(vector::length(&ban_registry) == 1, 1); + assert!(vector::length(&ban_registry) == 1, 2); + assert!(signer::address_of(validator_2) == leader_ban_registry::get_pool_address_from_vp(vector::borrow(&ban_registry, 0)), 3 ); + + // Let's say for round 5 validator 2 failed to propose again + leader_ban_registry::update_ban_registry(0, 5, option::some(2), vector[1]); + let ban_registry = leader_ban_registry::get_ban_registry(); + let vp = vector::borrow(&ban_registry, 0); + assert!(leader_ban_registry::get_consecutive_count_from_vp(vp) == 1, 4); + + // If on round 9 the validator 2 is able to propose then it will be removed from registry + leader_ban_registry::update_ban_registry(0, 9, option::some(1), vector[]); + let ban_registry = leader_ban_registry::get_ban_registry(); + assert!(vector::length(&ban_registry) == 0, 5); - aptos_std::debug::print(&ban_registry); - aptos_std::debug::print(&stake::get_committee_pool_addresses()); + // Now let's say validator 1 is unable to propose + leader_ban_registry::update_ban_registry(0, 12, option::some(1), vector[0]); + let ban_registry = leader_ban_registry::get_ban_registry(); + let vp = vector::borrow(&ban_registry, 0); + assert!(signer::address_of(validator_1) == leader_ban_registry::get_pool_address_from_vp(vp), 6 ); - assert!(signer::address_of(validator_2) == leader_ban_registry::get_pool_address_from_vp(vector::borrow(&ban_registry, 0)), 2 ); + leader_ban_registry::update_ban_registry(0, 12 + (4 * 1), option::some(1), vector[0]); + let ban_registry = leader_ban_registry::get_ban_registry(); + let vp = vector::borrow(&ban_registry, 0); + assert!(leader_ban_registry::get_consecutive_count_from_vp(vp) == 1, 8 ); + assert!(vector::length(&ban_registry) == 1, 9 ); - leader_ban_registry::update_ban_registry(0, 8, option::some(2), vector[1]); + leader_ban_registry::update_ban_registry(0, 12 + (4 * 2), option::some(1), vector[0]); let ban_registry = leader_ban_registry::get_ban_registry(); - aptos_std::debug::print(&ban_registry); + let vp = vector::borrow(&ban_registry, 0); + assert!(leader_ban_registry::get_consecutive_count_from_vp(vp) == 2, 8 ); + assert!(vector::length(&ban_registry) == 1, 9 ); + + leader_ban_registry::update_ban_registry(0, 12 + (4 * 3), option::some(1), vector[0]); + let ban_registry = leader_ban_registry::get_ban_registry(); + let vp = vector::borrow(&ban_registry, 0); + assert!(leader_ban_registry::get_consecutive_count_from_vp(vp) == 3, 8 ); + assert!(vector::length(&ban_registry) == 1, 9 ); + + leader_ban_registry::update_ban_registry(0, 12 + (4 * 4), option::some(1), vector[0]); + let ban_registry = leader_ban_registry::get_ban_registry(); + let vp = vector::borrow(&ban_registry, 0); + assert!(leader_ban_registry::get_consecutive_count_from_vp(vp) == 4, 8 ); + assert!(vector::length(&ban_registry) == 1, 9 ); + + // at 5th interval even though validator 1 is added a failed proposal index it will be removed from registry + // because max duration is set to 5 + leader_ban_registry::update_ban_registry(0, 12 + (4 * 5), option::some(1), vector[0]); + let ban_registry = leader_ban_registry::get_ban_registry(); + assert!(vector::length(&ban_registry) == 0, 10 ); + + // Now lets try to fail 3 of them + leader_ban_registry::update_ban_registry(0, 33, option::some(3), vector[0,1,2]); + let ban_registry = leader_ban_registry::get_ban_registry(); + // only 2 will be banned because of proposer limit of 2 + assert!(vector::length(&ban_registry) == 2, 11 ); + + leader_ban_registry::update_ban_registry(0, 33 + (4 * 1), option::some(2), vector[0,1]); + leader_ban_registry::update_ban_registry(0, 33 + (4 * 2), option::some(2), vector[0,1]); + leader_ban_registry::on_new_epoch(); // assuming every time this is called first before any epoch change + leader_ban_registry::update_ban_registry(1, 0, option::some(2), vector[0,1]); + leader_ban_registry::update_ban_registry(1, (4*1), option::some(2), vector[0,1]); + leader_ban_registry::update_ban_registry(1, (4*2), option::some(2), vector[0,1]); + leader_ban_registry::update_ban_registry(1, (4*3), option::some(2), vector[0,1]); + + let ban_registry = leader_ban_registry::get_ban_registry(); + // after epoch change and enough round the ban has been lifted correctly. + assert!(vector::length(&ban_registry) == 0, 12 ); + } } From 648c630c9f2030046f018eeaee5c87b558fa3cd0 Mon Sep 17 00:00:00 2001 From: Dhaval Purohit Date: Fri, 24 Oct 2025 19:05:45 +0530 Subject: [PATCH 11/14] resolved comments + formatted with movefmt --- .../configs/leader_ban_registry_config.move | 84 ++-- .../sources/leader_ban_registry.move | 358 ++++++++++-------- .../tests/test_leader_ban_registry.move | 156 ++++++-- .../test_leader_ban_registry_config.move | 76 +++- 4 files changed, 432 insertions(+), 242 deletions(-) diff --git a/aptos-move/framework/supra-framework/sources/configs/leader_ban_registry_config.move b/aptos-move/framework/supra-framework/sources/configs/leader_ban_registry_config.move index 8c4bfcca81ff5..f20a552faf8fb 100644 --- a/aptos-move/framework/supra-framework/sources/configs/leader_ban_registry_config.move +++ b/aptos-move/framework/supra-framework/sources/configs/leader_ban_registry_config.move @@ -27,7 +27,6 @@ module supra_framework::leader_ban_registry_config { /// The BanRegistryParameters already initialised const EALREADY_INITIALISED: u64 = 4; - /// Holds ban registry parameters bytes and it's version struct BanRegistryParameters has drop, key, store { /// Denotes config bcs bytes @@ -37,7 +36,7 @@ module supra_framework::leader_ban_registry_config { } /// Ban registry parameters v0 - struct BanRegistryParametersV0 has drop, key, store { + struct BanRegistryParametersV0 has drop, key, store { /// Denotes initial election count denied initial_elections_denied: u8, /// Denotes max election count denied @@ -47,10 +46,15 @@ module supra_framework::leader_ban_registry_config { } /// Publishes the BanRegistryParameters config. - public(friend) fun initialize(supra_framework: &signer, config: vector) { + public(friend) fun initialize( + supra_framework: &signer, config: vector + ) { system_addresses::assert_supra_framework(supra_framework); assert!(vector::length(&config) != 0, error::invalid_argument(EINVALID_CONFIG)); - assert!(!exists(@supra_framework), error::already_exists(EALREADY_INITIALISED)); + assert!( + !exists(@supra_framework), + error::already_exists(EALREADY_INITIALISED) + ); let v0 = deserialise_v0_params(config); if (option::is_none(&v0)) { abort error::invalid_argument(EINVALID_VERSION_BYTES) @@ -67,18 +71,28 @@ module supra_framework::leader_ban_registry_config { /// supra_framework::leader_ban_registry_config::set_for_next_epoch(&framework_signer, some_config_bytes, version); /// supra_framework::supra_governance::reconfigure(&framework_signer); /// ``` - public fun set_for_next_epoch(account: &signer, config: vector, version: u8) acquires BanRegistryParameters { + public fun set_for_next_epoch( + account: &signer, config: vector, version: u8 + ) acquires BanRegistryParameters { system_addresses::assert_supra_framework(account); assert!(vector::length(&config) != 0, error::invalid_argument(EINVALID_CONFIG)); if (exists(@supra_framework)) { - let ban_registry_params = borrow_global(@supra_framework); - assert!(version >= ban_registry_params.version, error::invalid_argument(EINVALID_VERSION)); + let ban_registry_params = + borrow_global(@supra_framework); + assert!( + version >= ban_registry_params.version, + error::invalid_argument(EINVALID_VERSION) + ); }; - std::config_buffer::upsert(BanRegistryParameters {config, version}); + std::config_buffer::upsert( + BanRegistryParameters { config, version } + ); } /// Only used in reconfigurations to apply the pending `BanRegistryParameters`, if there is any. - public(friend) fun on_new_epoch(framework: &signer) acquires BanRegistryParameters, BanRegistryParametersV0 { + public(friend) fun on_new_epoch( + framework: &signer + ) acquires BanRegistryParameters, BanRegistryParametersV0 { system_addresses::assert_supra_framework(framework); if (config_buffer::does_exist()) { let new_config = config_buffer::extract(); @@ -91,7 +105,7 @@ module supra_framework::leader_ban_registry_config { if (params.version == 0) { let v0 = deserialise_v0_params(params.config); // This is to prevent abort on new epoch if deserialise failed. - if (option::is_some(&v0)){ + if (option::is_some(&v0)) { let ban_registry_params = option::extract(&mut v0); if (exists(@supra_framework)) { *borrow_global_mut(@supra_framework) = ban_registry_params; @@ -107,11 +121,9 @@ module supra_framework::leader_ban_registry_config { #[view] public fun get_ban_registry_params(): (vector, u8) acquires BanRegistryParameters { if (exists(@supra_framework)) { - let ban_registry_config = borrow_global(@supra_framework); - return ( - ban_registry_config.config, - ban_registry_config.version - ) + let ban_registry_config = + borrow_global(@supra_framework); + return (ban_registry_config.config, ban_registry_config.version) }; (vector::empty(), 0) } @@ -119,9 +131,11 @@ module supra_framework::leader_ban_registry_config { #[view] public fun get_ban_registry_params_v0(): (u8, u32, u8) acquires BanRegistryParameters, BanRegistryParametersV0 { if (exists(@supra_framework)) { - let ban_registry_config = borrow_global(@supra_framework); + let ban_registry_config = + borrow_global(@supra_framework); if (ban_registry_config.version == 0) { - let ban_registry_params = borrow_global(@supra_framework); + let ban_registry_params = + borrow_global(@supra_framework); return ( ban_registry_params.initial_elections_denied, ban_registry_params.max_elections_denied, @@ -135,11 +149,13 @@ module supra_framework::leader_ban_registry_config { /// Provide initial election denied value public fun get_initial_elections_denied(): u8 acquires BanRegistryParameters, BanRegistryParametersV0 { if (exists(@supra_framework)) { - let ban_registry_config = borrow_global(@supra_framework); + let ban_registry_config = + borrow_global(@supra_framework); if (ban_registry_config.version == 0) { - let ban_registry_params = borrow_global(@supra_framework); + let ban_registry_params = + borrow_global(@supra_framework); return ban_registry_params.initial_elections_denied - } + } }; 0 } @@ -147,11 +163,13 @@ module supra_framework::leader_ban_registry_config { /// Provide max election denied value public fun get_max_elections_denied(): u32 acquires BanRegistryParameters, BanRegistryParametersV0 { if (exists(@supra_framework)) { - let ban_registry_config = borrow_global(@supra_framework); + let ban_registry_config = + borrow_global(@supra_framework); if (ban_registry_config.version == 0) { - let ban_registry_params = borrow_global(@supra_framework); + let ban_registry_params = + borrow_global(@supra_framework); return ban_registry_params.max_elections_denied - } + } }; 0 } @@ -159,11 +177,13 @@ module supra_framework::leader_ban_registry_config { /// Provide minimum unbanned proposers value public fun get_minimum_unbanned_proposers(): u8 acquires BanRegistryParameters, BanRegistryParametersV0 { if (exists(@supra_framework)) { - let ban_registry_config = borrow_global(@supra_framework); + let ban_registry_config = + borrow_global(@supra_framework); if (ban_registry_config.version == 0) { - let ban_registry_params = borrow_global(@supra_framework); + let ban_registry_params = + borrow_global(@supra_framework); return ban_registry_params.minimum_unbanned_proposers - } + } }; 0 } @@ -176,11 +196,13 @@ module supra_framework::leader_ban_registry_config { let minimum_unbanned_proposers: u8 = decode_bcs::peel_u8(&mut bcs_bytes); // making sure no bytes left to decode means correct parameter version if (vector::length(&decode_bcs::into_remainder_bytes(bcs_bytes)) == 0) { - return option::some(BanRegistryParametersV0 { - initial_elections_denied, - max_elections_denied, - minimum_unbanned_proposers - }) + return option::some( + BanRegistryParametersV0 { + initial_elections_denied, + max_elections_denied, + minimum_unbanned_proposers + } + ) }; option::none() } diff --git a/aptos-move/framework/supra-framework/sources/leader_ban_registry.move b/aptos-move/framework/supra-framework/sources/leader_ban_registry.move index 2830a2914a962..bd4e9b1fe0cfb 100644 --- a/aptos-move/framework/supra-framework/sources/leader_ban_registry.move +++ b/aptos-move/framework/supra-framework/sources/leader_ban_registry.move @@ -25,11 +25,11 @@ module supra_framework::leader_ban_registry { /// Latest view already initialized const ELATEST_VIEW_ALREADY_EXISTS: u64 = 3; - /// Holds metrics for banned round, epoch and round server in prev epoch + /// Holds metrics for banned round, epoch and round served in prev epoch struct ActiveBan has store, drop, copy { - /// Epoch when validator first got banned + /// Epoch in which the ban was issued epoch_earned: u64, - /// Round when validator first got banned + /// Round in which the ban was issued round_earned: u64, /// Round count incremented on every epoch change rounds_served_in_previous_epochs: u64 @@ -61,7 +61,7 @@ module supra_framework::leader_ban_registry { #[event] /// Emits when validator receives a ban or consucutive ban occurred - struct Bannned has drop, store { + struct Banned has drop, store { /// Validator's pool address pool_address: address, /// Epoch @@ -84,7 +84,9 @@ module supra_framework::leader_ban_registry { } /// Initialise leader ban registry - public(friend) fun initialize_leader_ban_registry(supra_framework: &signer) { + public(friend) fun initialize_leader_ban_registry( + supra_framework: &signer + ) { system_addresses::assert_supra_framework(supra_framework); assert!( !exists(@supra_framework), @@ -100,7 +102,7 @@ module supra_framework::leader_ban_registry { #[view] /// Returns list of validators active ban with it's pool address - public fun get_ban_registry() : vector acquires BanRegistry { + public fun get_ban_registry(): vector acquires BanRegistry { if (!exists(@supra_framework)) { return vector::empty() }; @@ -110,13 +112,9 @@ module supra_framework::leader_ban_registry { #[view] /// Return initial ban duration - public fun get_latest_view(): LatestView - acquires LatestView { + public fun get_latest_view(): LatestView acquires LatestView { if (!exists(@supra_framework)) { - return LatestView { - epoch: 0, - round: 0 - } + return LatestView { epoch: 0, round: 0 } }; let latest_view = borrow_global(@supra_framework); *latest_view @@ -125,7 +123,8 @@ module supra_framework::leader_ban_registry { #[view] /// Return initial ban duration public fun get_initial_ban_duration(): u64 { - let initial_elections_denied = leader_ban_registry_config::get_initial_elections_denied(); + let initial_elections_denied = + leader_ban_registry_config::get_initial_elections_denied(); let committee_size = stake::get_committee_size(); committee_size * (initial_elections_denied as u64) } @@ -140,22 +139,19 @@ module supra_framework::leader_ban_registry { /// Add or update the ban registry as per block metadata public(friend) fun update_ban_registry( - epoch_earned: u64, round_earned: u64, proposer_index: Option, failed_proposer_indices: vector - ) - acquires BanRegistry, LatestView - { - if (!exists(@supra_framework)) { - return - }; - if (!exists(@supra_framework)) { - return - }; + current_epoch: u64, + current_round: u64, + proposer_index: Option, + failed_proposer_indices: vector + ) acquires BanRegistry, LatestView { + if (!exists(@supra_framework)) { return }; + if (!exists(@supra_framework)) { return }; let ban_registry = borrow_global_mut(@supra_framework); let latest_view = borrow_global_mut(@supra_framework); - latest_view.epoch = epoch_earned; - latest_view.round = round_earned; + latest_view.epoch = current_epoch; + latest_view.round = current_round; - // ban the failed proposer + // ban the failed proposers ban_failed_proposers(latest_view, failed_proposer_indices, ban_registry); // remove expired bans @@ -172,105 +168,141 @@ module supra_framework::leader_ban_registry { fun ban_failed_proposers( latest_view: &LatestView, failed_proposer_indices: vector, - ban_registry: &mut BanRegistry, - ) - { + ban_registry: &mut BanRegistry + ) { let initial_ban_duration = get_initial_ban_duration(); - if (initial_ban_duration == 0) { - return - }; - - vector::for_each(failed_proposer_indices, |failed_validator_index| { - let validator_pool_address_opt = stake::get_pool_address_from_index(failed_validator_index); - if (option::is_some(&validator_pool_address_opt)) { - let validator_pool_address = option::extract(&mut validator_pool_address_opt); - let (is_banned, index) = vector::find(&ban_registry.bans, |v| { - let v: &ValidatorBansWithAddress = v; - validator_pool_address == v.pool_address - }); - if (is_banned) { - let bans = vector::borrow_mut(&mut ban_registry.bans, index); - bans.consecutive_bans = bans.consecutive_bans + 1; - if (features::module_event_enabled()) { - event::emit(Bannned { - pool_address: validator_pool_address, - epoch: latest_view.epoch, - round: latest_view.round, - consecutive_bans: bans.consecutive_bans - }); - } - } else { - let ban_registry_len = vector::length(&ban_registry.bans); - if (can_be_banned(ban_registry_len)) { - let ban_with_address = ValidatorBansWithAddress { - active: ActiveBan { - epoch_earned: latest_view.epoch, - round_earned: latest_view.round, - rounds_served_in_previous_epochs: 0 - }, - consecutive_bans: 0, - pool_address: validator_pool_address - }; - vector::push_back(&mut ban_registry.bans, ban_with_address); - + if (initial_ban_duration == 0) { return }; + + vector::for_each( + failed_proposer_indices, + |failed_validator_index| { + let validator_pool_address_opt = + stake::get_pool_address_from_index(failed_validator_index); + if (option::is_some(&validator_pool_address_opt)) { + let validator_pool_address = + option::extract(&mut validator_pool_address_opt); + let (is_banned, index) = vector::find( + &ban_registry.bans, + |v| { + let v: &ValidatorBansWithAddress = v; + validator_pool_address == v.pool_address + } + ); + if (is_banned) { + let bans = vector::borrow_mut(&mut ban_registry.bans, index); + bans.consecutive_bans = bans.consecutive_bans + 1; + // bans.active.round_earned = latest_view.round; + // bans.active.epoch_earned = latest_view.epoch; + // bans.active.rounds_served_in_previous_epochs = 0; if (features::module_event_enabled()) { - event::emit(Bannned { - pool_address: ban_with_address.pool_address, - epoch: ban_with_address.active.epoch_earned, - round: ban_with_address.active.round_earned, - consecutive_bans: ban_with_address.consecutive_bans - }); + event::emit( + Banned { + pool_address: validator_pool_address, + epoch: latest_view.epoch, + round: latest_view.round, + consecutive_bans: bans.consecutive_bans + } + ); } - } + } else { + let ban_registry_len = vector::length(&ban_registry.bans); + if (can_be_banned(ban_registry_len)) { + let ban_with_address = ValidatorBansWithAddress { + active: ActiveBan { + epoch_earned: latest_view.epoch, + round_earned: latest_view.round, + rounds_served_in_previous_epochs: 0 + }, + consecutive_bans: 0, + pool_address: validator_pool_address + }; + vector::push_back(&mut ban_registry.bans, ban_with_address); + + if (features::module_event_enabled()) { + event::emit( + Banned { + pool_address: ban_with_address.pool_address, + epoch: ban_with_address.active.epoch_earned, + round: ban_with_address.active.round_earned, + consecutive_bans: ban_with_address.consecutive_bans + } + ); + } + } + }; }; - }; - }); + } + ); } /// Removes bans for those validator which duration is over - fun reinstate_expired_bans(latest_view: &LatestView, ban_registry: &mut BanRegistry) { + fun reinstate_expired_bans( + latest_view: &LatestView, ban_registry: &mut BanRegistry + ) { let pool_address_for_duration_over = vector::empty(); - vector::for_each_ref(&ban_registry.bans, |v| { - if (remaining_duration(v, latest_view) == 0) { - vector::push_back(&mut pool_address_for_duration_over, v.pool_address); + vector::for_each_ref( + &ban_registry.bans, + |v| { + if (remaining_duration(v, latest_view) == 0) { + vector::push_back( + &mut pool_address_for_duration_over, v.pool_address + ); + } } - }); - vector::for_each_ref(&pool_address_for_duration_over, |p| { - let (is_banned, index) = vector::find(&ban_registry.bans, |v| { - let v : &ValidatorBansWithAddress = v; - &v.pool_address == p - }); - if (is_banned) { - vector::swap_remove(&mut ban_registry.bans, index); - if (features::module_event_enabled()) { - event::emit(Reinstated { - epoch: latest_view.epoch, - round: latest_view.round, - pool_address: *p - }) + ); + vector::for_each_ref( + &pool_address_for_duration_over, + |p| { + let (is_banned, index) = vector::find( + &ban_registry.bans, + |v| { + let v: &ValidatorBansWithAddress = v; + &v.pool_address == p + } + ); + if (is_banned) { + vector::swap_remove(&mut ban_registry.bans, index); + if (features::module_event_enabled()) { + event::emit( + Reinstated { + epoch: latest_view.epoch, + round: latest_view.round, + pool_address: *p + } + ) + } } } - }); + ); } /// Remvoing an entry of validator from registry if found at proposer index - fun reinstate_proposer(latest_view: &LatestView, proposer_index: u64, ban_registry: &mut BanRegistry) - { - let validator_pool_address_opt = stake::get_pool_address_from_index(proposer_index); + fun reinstate_proposer( + latest_view: &LatestView, proposer_index: u64, ban_registry: &mut BanRegistry + ) { + let validator_pool_address_opt = + stake::get_pool_address_from_index(proposer_index); if (option::is_some(&validator_pool_address_opt)) { - let validator_pool_address = option::extract(&mut validator_pool_address_opt); - let (is_banned, index) = vector::find(&ban_registry.bans, |v| { - let v: &ValidatorBansWithAddress = v; - validator_pool_address == v.pool_address - }); + let validator_pool_address = option::extract( + &mut validator_pool_address_opt + ); + let (is_banned, index) = vector::find( + &ban_registry.bans, + |v| { + let v: &ValidatorBansWithAddress = v; + validator_pool_address == v.pool_address + } + ); if (is_banned) { vector::swap_remove(&mut ban_registry.bans, index); if (features::module_event_enabled()) { - event::emit(Reinstated { - round: latest_view.round, - epoch: latest_view.epoch, - pool_address: validator_pool_address - }) + event::emit( + Reinstated { + round: latest_view.round, + epoch: latest_view.epoch, + pool_address: validator_pool_address + } + ) } }; }; @@ -278,13 +310,9 @@ module supra_framework::leader_ban_registry { /// Update counts on every epoch /// Only run after committee has been updated from reconfigure - public(friend) fun on_new_epoch() acquires BanRegistry,LatestView { - if (!exists(@supra_framework)) { - return - }; - if (!exists(@supra_framework)) { - return - }; + public(friend) fun on_new_epoch() acquires BanRegistry, LatestView { + if (!exists(@supra_framework)) { return }; + if (!exists(@supra_framework)) { return }; let latest_view = borrow_global(@supra_framework); let ban_registry = borrow_global_mut(@supra_framework); @@ -292,65 +320,83 @@ module supra_framework::leader_ban_registry { let new_committee_pool_addresses = stake::get_committee_pool_addresses(); // The pool addresses of the validators that have left the committee. let retired_validators = vector::empty(); - vector::for_each_mut(&mut ban_registry.bans, |v| { - let v: &mut ValidatorBansWithAddress = v; - if (has_started(&v.active, latest_view)) { - if (latest_view.epoch > v.active.epoch_earned) { - v.active.rounds_served_in_previous_epochs = v.active.rounds_served_in_previous_epochs + latest_view.round; - } else { - v.active.rounds_served_in_previous_epochs = v.active.rounds_served_in_previous_epochs + latest_view.round - v.active.round_earned; + vector::for_each_mut( + &mut ban_registry.bans, + |v| { + let v: &mut ValidatorBansWithAddress = v; + if (has_started(&v.active, latest_view)) { + if (latest_view.epoch > v.active.epoch_earned) { + v.active.rounds_served_in_previous_epochs = v.active.rounds_served_in_previous_epochs + + latest_view.round; + } else { + v.active.rounds_served_in_previous_epochs = v.active.rounds_served_in_previous_epochs + + latest_view.round - v.active.round_earned; + } + }; + if (!vector::contains(&new_committee_pool_addresses, &v.pool_address)) { + vector::push_back(&mut retired_validators, v.pool_address) } - }; - if (!vector::contains(&new_committee_pool_addresses, &v.pool_address)) { - vector::push_back(&mut retired_validators, v.pool_address) } - }); + ); - vector::for_each_ref(&retired_validators, |p| { - let (is_banned, index) = vector::find(&ban_registry.bans, |v|{ - let v : &ValidatorBansWithAddress = v; - &v.pool_address == p - }); - if (is_banned) { - vector::swap_remove(&mut ban_registry.bans, index); - if (features::module_event_enabled()) { - event::emit(Reinstated { - pool_address: *p, - epoch: latest_view.epoch, - round: latest_view.round - }); + vector::for_each_ref( + &retired_validators, + |p| { + let (is_banned, index) = vector::find( + &ban_registry.bans, + |v| { + let v: &ValidatorBansWithAddress = v; + &v.pool_address == p + } + ); + if (is_banned) { + vector::swap_remove(&mut ban_registry.bans, index); + if (features::module_event_enabled()) { + event::emit( + Reinstated { + pool_address: *p, + epoch: latest_view.epoch, + round: latest_view.round + } + ); + } } } - }); + ); } /// Checks if ban has already started fun has_started(active_ban: &ActiveBan, latest_view: &LatestView): bool { - active_ban.epoch_earned < latest_view.epoch || - (active_ban.epoch_earned == latest_view.epoch && active_ban.round_earned < latest_view.round) + active_ban.epoch_earned < latest_view.epoch + || ( + active_ban.epoch_earned == latest_view.epoch + && active_ban.round_earned < latest_view.round + ) } /// Calculate the number of rounds remaining in the a given ban. - fun remaining_duration(ban: &ValidatorBansWithAddress, latest_view: &LatestView): u64 { + fun remaining_duration( + ban: &ValidatorBansWithAddress, latest_view: &LatestView + ): u64 { let initial_ban_duration = get_initial_ban_duration(); let max_ban_duration = get_max_ban_duration(); let duration = initial_ban_duration * pow(2, (ban.consecutive_bans as u64)); let duration = min(duration, max_ban_duration); - let rounds_served = if (latest_view.epoch > ban.active.epoch_earned) { - ban.active.rounds_served_in_previous_epochs + latest_view.round - } else { - latest_view.round - ban.active.round_earned - }; + let rounds_served = + if (latest_view.epoch > ban.active.epoch_earned) { + ban.active.rounds_served_in_previous_epochs + latest_view.round + } else { + latest_view.round - ban.active.round_earned + }; if (duration >= rounds_served) { duration - rounds_served - } else { - 0 - } + } else { 0 } } /// Returns true until ban registry size + minimum proposers required count less than committee size - fun can_be_banned(ban_registry_len: u64) : bool { - let minimum_unbanned_proposers = leader_ban_registry_config::get_minimum_unbanned_proposers(); + fun can_be_banned(ban_registry_len: u64): bool { + let minimum_unbanned_proposers = + leader_ban_registry_config::get_minimum_unbanned_proposers(); let committee_size = stake::get_committee_size(); committee_size > ban_registry_len + (minimum_unbanned_proposers as u64) } @@ -364,12 +410,16 @@ module supra_framework::leader_ban_registry { } #[test_only] - public fun get_pool_address_from_vp(validator_with_pool_addr: &ValidatorBansWithAddress) : address { + public fun get_pool_address_from_vp( + validator_with_pool_addr: &ValidatorBansWithAddress + ): address { validator_with_pool_addr.pool_address } #[test_only] - public fun get_consecutive_count_from_vp(validator_with_pool_addr: &ValidatorBansWithAddress) : u32 { + public fun get_consecutive_count_from_vp( + validator_with_pool_addr: &ValidatorBansWithAddress + ): u32 { validator_with_pool_addr.consecutive_bans } } diff --git a/aptos-move/framework/supra-framework/tests/test_leader_ban_registry.move b/aptos-move/framework/supra-framework/tests/test_leader_ban_registry.move index 50bfc7d8f1a92..c529c45f4aaae 100644 --- a/aptos-move/framework/supra-framework/tests/test_leader_ban_registry.move +++ b/aptos-move/framework/supra-framework/tests/test_leader_ban_registry.move @@ -45,27 +45,75 @@ module std::test_leader_ban_registry { let active_coin = coin::withdraw(validator_1, 500); let pending_coin = coin::withdraw(validator_1, 1000); - stake::create_stake_pool(validator_1, active_coin, pending_coin, 500000000); + stake::create_stake_pool( + validator_1, + active_coin, + pending_coin, + 500000000 + ); let active_coin = coin::withdraw(validator_2, 500); let pending_coin = coin::withdraw(validator_2, 1000); - stake::create_stake_pool(validator_2, active_coin, pending_coin, 500000000); + stake::create_stake_pool( + validator_2, + active_coin, + pending_coin, + 500000000 + ); let active_coin = coin::withdraw(validator_3, 500); let pending_coin = coin::withdraw(validator_3, 1000); - stake::create_stake_pool(validator_3, active_coin, pending_coin, 500000000); + stake::create_stake_pool( + validator_3, + active_coin, + pending_coin, + 500000000 + ); let active_coin = coin::withdraw(validator_4, 500); let pending_coin = coin::withdraw(validator_4, 1000); - stake::create_stake_pool(validator_4, active_coin, pending_coin, 500000000); - - stake::join_validator_set_for_test(&ed25519::public_key_to_unvalidated(&v_4_p_key), validator_4, signer::address_of(validator_4), false); - stake::join_validator_set_for_test(&ed25519::public_key_to_unvalidated(&v_3_p_key), validator_3,signer::address_of(validator_3), false); - stake::join_validator_set_for_test(&ed25519::public_key_to_unvalidated(&v_2_p_key), validator_2, signer::address_of(validator_2), false); - stake::join_validator_set_for_test(&ed25519::public_key_to_unvalidated(&v_1_p_key), validator_1, signer::address_of(validator_1), true); // on epoch change + stake::create_stake_pool( + validator_4, + active_coin, + pending_coin, + 500000000 + ); + + stake::join_validator_set_for_test( + &ed25519::public_key_to_unvalidated(&v_4_p_key), + validator_4, + signer::address_of(validator_4), + false + ); + stake::join_validator_set_for_test( + &ed25519::public_key_to_unvalidated(&v_3_p_key), + validator_3, + signer::address_of(validator_3), + false + ); + stake::join_validator_set_for_test( + &ed25519::public_key_to_unvalidated(&v_2_p_key), + validator_2, + signer::address_of(validator_2), + false + ); + stake::join_validator_set_for_test( + &ed25519::public_key_to_unvalidated(&v_1_p_key), + validator_1, + signer::address_of(validator_1), + true + ); // on epoch change } - #[test(sender = @supra_framework, validator_1 = @0xdead01, validator_2 = @0xdead02, validator_3 = @0xdead03, validator_4 = @0xdead04)] + #[ + test( + sender = @supra_framework, + validator_1 = @0xdead01, + validator_2 = @0xdead02, + validator_3 = @0xdead03, + validator_4 = @0xdead04 + ) + ] fun test_ban_registy_e2e( sender: &signer, validator_1: &signer, @@ -76,11 +124,17 @@ module std::test_leader_ban_registry { // initialise ban registry config let ban_params = leader_ban_registry_config::get_test_ban_registry_params_v0(); let ban_config_bytes = bcs::to_bytes(&ban_params); - leader_ban_registry_config::initialize(sender,ban_config_bytes); + leader_ban_registry_config::initialize(sender, ban_config_bytes); // initialise ban registry leader_ban_registry::initialize_leader_ban_registry(sender); - setup_staking_modules(sender, validator_1, validator_2, validator_3, validator_4); + setup_staking_modules( + sender, + validator_1, + validator_2, + validator_3, + validator_4 + ); // update ban registry leader_ban_registry::update_ban_registry(0, 0, option::some(0), vector::empty()); @@ -91,7 +145,13 @@ module std::test_leader_ban_registry { leader_ban_registry::update_ban_registry(0, 1, option::some(2), vector[1]); let ban_registry = leader_ban_registry::get_ban_registry(); assert!(vector::length(&ban_registry) == 1, 2); - assert!(signer::address_of(validator_2) == leader_ban_registry::get_pool_address_from_vp(vector::borrow(&ban_registry, 0)), 3 ); + assert!( + signer::address_of(validator_2) + == leader_ban_registry::get_pool_address_from_vp( + vector::borrow(&ban_registry, 0) + ), + 3 + ); // Let's say for round 5 validator 2 failed to propose again leader_ban_registry::update_ban_registry(0, 5, option::some(2), vector[1]); @@ -108,55 +168,73 @@ module std::test_leader_ban_registry { leader_ban_registry::update_ban_registry(0, 12, option::some(1), vector[0]); let ban_registry = leader_ban_registry::get_ban_registry(); let vp = vector::borrow(&ban_registry, 0); - assert!(signer::address_of(validator_1) == leader_ban_registry::get_pool_address_from_vp(vp), 6 ); - - leader_ban_registry::update_ban_registry(0, 12 + (4 * 1), option::some(1), vector[0]); + assert!( + signer::address_of(validator_1) + == leader_ban_registry::get_pool_address_from_vp(vp), + 6 + ); + + leader_ban_registry::update_ban_registry( + 0, 12 + (4 * 1), option::some(1), vector[0] + ); let ban_registry = leader_ban_registry::get_ban_registry(); let vp = vector::borrow(&ban_registry, 0); - assert!(leader_ban_registry::get_consecutive_count_from_vp(vp) == 1, 8 ); - assert!(vector::length(&ban_registry) == 1, 9 ); + assert!(leader_ban_registry::get_consecutive_count_from_vp(vp) == 1, 8); + assert!(vector::length(&ban_registry) == 1, 9); - leader_ban_registry::update_ban_registry(0, 12 + (4 * 2), option::some(1), vector[0]); + leader_ban_registry::update_ban_registry( + 0, 12 + (4 * 2), option::some(1), vector[0] + ); let ban_registry = leader_ban_registry::get_ban_registry(); let vp = vector::borrow(&ban_registry, 0); - assert!(leader_ban_registry::get_consecutive_count_from_vp(vp) == 2, 8 ); - assert!(vector::length(&ban_registry) == 1, 9 ); + assert!(leader_ban_registry::get_consecutive_count_from_vp(vp) == 2, 8); + assert!(vector::length(&ban_registry) == 1, 9); - leader_ban_registry::update_ban_registry(0, 12 + (4 * 3), option::some(1), vector[0]); + leader_ban_registry::update_ban_registry( + 0, 12 + (4 * 3), option::some(1), vector[0] + ); let ban_registry = leader_ban_registry::get_ban_registry(); let vp = vector::borrow(&ban_registry, 0); - assert!(leader_ban_registry::get_consecutive_count_from_vp(vp) == 3, 8 ); - assert!(vector::length(&ban_registry) == 1, 9 ); + assert!(leader_ban_registry::get_consecutive_count_from_vp(vp) == 3, 8); + assert!(vector::length(&ban_registry) == 1, 9); - leader_ban_registry::update_ban_registry(0, 12 + (4 * 4), option::some(1), vector[0]); + leader_ban_registry::update_ban_registry( + 0, 12 + (4 * 4), option::some(1), vector[0] + ); let ban_registry = leader_ban_registry::get_ban_registry(); let vp = vector::borrow(&ban_registry, 0); - assert!(leader_ban_registry::get_consecutive_count_from_vp(vp) == 4, 8 ); - assert!(vector::length(&ban_registry) == 1, 9 ); + assert!(leader_ban_registry::get_consecutive_count_from_vp(vp) == 4, 8); + assert!(vector::length(&ban_registry) == 1, 9); // at 5th interval even though validator 1 is added a failed proposal index it will be removed from registry // because max duration is set to 5 - leader_ban_registry::update_ban_registry(0, 12 + (4 * 5), option::some(1), vector[0]); + leader_ban_registry::update_ban_registry( + 0, 12 + (4 * 5), option::some(1), vector[0] + ); let ban_registry = leader_ban_registry::get_ban_registry(); - assert!(vector::length(&ban_registry) == 0, 10 ); + assert!(vector::length(&ban_registry) == 0, 10); // Now lets try to fail 3 of them - leader_ban_registry::update_ban_registry(0, 33, option::some(3), vector[0,1,2]); + leader_ban_registry::update_ban_registry(0, 33, option::some(3), vector[0, 1, 2]); let ban_registry = leader_ban_registry::get_ban_registry(); // only 2 will be banned because of proposer limit of 2 - assert!(vector::length(&ban_registry) == 2, 11 ); - - leader_ban_registry::update_ban_registry(0, 33 + (4 * 1), option::some(2), vector[0,1]); - leader_ban_registry::update_ban_registry(0, 33 + (4 * 2), option::some(2), vector[0,1]); + assert!(vector::length(&ban_registry) == 2, 11); + + leader_ban_registry::update_ban_registry( + 0, 33 + (4 * 1), option::some(2), vector[0, 1] + ); + leader_ban_registry::update_ban_registry( + 0, 33 + (4 * 2), option::some(2), vector[0, 1] + ); leader_ban_registry::on_new_epoch(); // assuming every time this is called first before any epoch change - leader_ban_registry::update_ban_registry(1, 0, option::some(2), vector[0,1]); - leader_ban_registry::update_ban_registry(1, (4*1), option::some(2), vector[0,1]); - leader_ban_registry::update_ban_registry(1, (4*2), option::some(2), vector[0,1]); - leader_ban_registry::update_ban_registry(1, (4*3), option::some(2), vector[0,1]); + leader_ban_registry::update_ban_registry(1, 0, option::some(2), vector[0, 1]); + leader_ban_registry::update_ban_registry(1, (4 * 1), option::some(2), vector[0, 1]); + leader_ban_registry::update_ban_registry(1, (4 * 2), option::some(2), vector[0, 1]); + leader_ban_registry::update_ban_registry(1, (4 * 3), option::some(2), vector[0, 1]); let ban_registry = leader_ban_registry::get_ban_registry(); // after epoch change and enough round the ban has been lifted correctly. - assert!(vector::length(&ban_registry) == 0, 12 ); + assert!(vector::length(&ban_registry) == 0, 12); } } diff --git a/aptos-move/framework/supra-framework/tests/test_leader_ban_registry_config.move b/aptos-move/framework/supra-framework/tests/test_leader_ban_registry_config.move index 6227af230ad13..c248a20ca9a36 100644 --- a/aptos-move/framework/supra-framework/tests/test_leader_ban_registry_config.move +++ b/aptos-move/framework/supra-framework/tests/test_leader_ban_registry_config.move @@ -6,31 +6,45 @@ module supra_framework::test_leader_ban_registry_config { use supra_framework::leader_ban_registry_config; #[test(sender = @0xdead)] - #[expected_failure(abort_code=0x50003, location=supra_framework::system_addresses)] // signer is not valid + #[expected_failure(abort_code = 0x50003, location = supra_framework::system_addresses)] + // signer is not valid fun test_signer(sender: &signer) { - let ban_registry_param = leader_ban_registry_config::get_test_ban_registry_params_v0(); + let ban_registry_param = + leader_ban_registry_config::get_test_ban_registry_params_v0(); let config_bytes = bcs::to_bytes(&ban_registry_param); leader_ban_registry_config::initialize(sender, config_bytes); } #[test(sender = @supra_framework)] - #[expected_failure(abort_code=0x10001, location=supra_framework::leader_ban_registry_config)] // Invalid config bytes + #[ + expected_failure( + abort_code = 0x10001, location = supra_framework::leader_ban_registry_config + ) + ] + // Invalid config bytes fun test_empty_config_value(sender: &signer) { let config_bytes = vector::empty(); leader_ban_registry_config::initialize(sender, config_bytes); } #[test(sender = @supra_framework)] - #[expected_failure(abort_code=0x10003, location=supra_framework::leader_ban_registry_config)] // Invalid version bytes + #[ + expected_failure( + abort_code = 0x10003, location = supra_framework::leader_ban_registry_config + ) + ] + // Invalid version bytes fun test_invalid_config_value(sender: &signer) { - let ban_registry_param = leader_ban_registry_config::get_test_ban_registry_params_v0(); + let ban_registry_param = + leader_ban_registry_config::get_test_ban_registry_params_v0(); let config_bytes = bcs::to_bytes(&ban_registry_param); vector::push_back(&mut config_bytes, 0); leader_ban_registry_config::initialize(sender, config_bytes); } public fun init_ban_registry_params(sender: &signer) { - let ban_registry_param = leader_ban_registry_config::get_test_ban_registry_params_v0(); + let ban_registry_param = + leader_ban_registry_config::get_test_ban_registry_params_v0(); let config_bytes = bcs::to_bytes(&ban_registry_param); leader_ban_registry_config::initialize(sender, config_bytes); } @@ -39,11 +53,18 @@ module supra_framework::test_leader_ban_registry_config { public fun test_init_config_value(sender: &signer) { init_ban_registry_params(sender); assert!(leader_ban_registry_config::check_ban_registry_params_exist(sender), 1); - assert!(leader_ban_registry_config::check_ban_registry_params_v0_exist(sender), 2); + assert!( + leader_ban_registry_config::check_ban_registry_params_v0_exist(sender), 2 + ); } #[test(sender = @supra_framework)] - #[expected_failure(abort_code=0x80004, location=supra_framework::leader_ban_registry_config)] // Already initialised + #[ + expected_failure( + abort_code = 0x80004, location = supra_framework::leader_ban_registry_config + ) + ] + // Already initialised fun test_init_config_value_reinit(sender: &signer) { init_ban_registry_params(sender); init_ban_registry_params(sender); @@ -52,27 +73,46 @@ module supra_framework::test_leader_ban_registry_config { #[test(sender = @supra_framework)] fun test_on_epoch_change(sender: &signer) { config_buffer::initialize(sender); - assert!(!leader_ban_registry_config::check_ban_registry_params_exist(sender), 1); - let ban_registry_param = leader_ban_registry_config::get_test_ban_registry_params_v0(); + assert!( + !leader_ban_registry_config::check_ban_registry_params_exist(sender), 1 + ); + let ban_registry_param = + leader_ban_registry_config::get_test_ban_registry_params_v0(); let config_bytes = bcs::to_bytes(&ban_registry_param); leader_ban_registry_config::set_for_next_epoch(sender, config_bytes, 0); leader_ban_registry_config::on_new_epoch(sender); assert!(leader_ban_registry_config::check_ban_registry_params_exist(sender), 1); - assert!(leader_ban_registry_config::check_ban_registry_params_v0_exist(sender), 2); + assert!( + leader_ban_registry_config::check_ban_registry_params_v0_exist(sender), 2 + ); let updated_initial_e_denied = 3; let updated_max_e_denied = 8; let updated_minimum_u_proposers = 6; - let ban_registry_param = leader_ban_registry_config::get_custom_ban_registry_params_v0( - updated_initial_e_denied, - updated_max_e_denied, - updated_minimum_u_proposers); + let ban_registry_param = + leader_ban_registry_config::get_custom_ban_registry_params_v0( + updated_initial_e_denied, + updated_max_e_denied, + updated_minimum_u_proposers + ); let config_bytes = bcs::to_bytes(&ban_registry_param); leader_ban_registry_config::set_for_next_epoch(sender, config_bytes, 0); leader_ban_registry_config::on_new_epoch(sender); - assert!(leader_ban_registry_config::get_initial_elections_denied()==updated_initial_e_denied, 3); - assert!(leader_ban_registry_config::get_max_elections_denied()==updated_max_e_denied, 4); - assert!(leader_ban_registry_config::get_minimum_unbanned_proposers()==updated_minimum_u_proposers, 5); + assert!( + leader_ban_registry_config::get_initial_elections_denied() + == updated_initial_e_denied, + 3 + ); + assert!( + leader_ban_registry_config::get_max_elections_denied() + == updated_max_e_denied, + 4 + ); + assert!( + leader_ban_registry_config::get_minimum_unbanned_proposers() + == updated_minimum_u_proposers, + 5 + ); } } From 023e4968bf8944c51516cea1a0339e36326355a2 Mon Sep 17 00:00:00 2001 From: Dhaval Purohit Date: Thu, 5 Feb 2026 18:25:09 +0530 Subject: [PATCH 12/14] updated with probation period --- .../configs/leader_ban_registry_config.move | 38 +++-- .../sources/leader_ban_registry.move | 130 +++++++++++++++--- .../tests/test_leader_ban_registry.move | 118 +++++++++++++--- .../test_leader_ban_registry_config.move | 9 +- 4 files changed, 253 insertions(+), 42 deletions(-) diff --git a/aptos-move/framework/supra-framework/sources/configs/leader_ban_registry_config.move b/aptos-move/framework/supra-framework/sources/configs/leader_ban_registry_config.move index f20a552faf8fb..87fe77a1beab4 100644 --- a/aptos-move/framework/supra-framework/sources/configs/leader_ban_registry_config.move +++ b/aptos-move/framework/supra-framework/sources/configs/leader_ban_registry_config.move @@ -42,7 +42,9 @@ module supra_framework::leader_ban_registry_config { /// Denotes max election count denied max_elections_denied: u32, /// Denotes minimum unbanned proposer count - minimum_unbanned_proposers: u8 + minimum_unbanned_proposers: u8, + /// Denotes the number of elections a validator must serve on probation after ban expires + probation_elections: u8 } /// Publishes the BanRegistryParameters config. @@ -129,7 +131,7 @@ module supra_framework::leader_ban_registry_config { } #[view] - public fun get_ban_registry_params_v0(): (u8, u32, u8) acquires BanRegistryParameters, BanRegistryParametersV0 { + public fun get_ban_registry_params_v0(): (u8, u32, u8, u8) acquires BanRegistryParameters, BanRegistryParametersV0 { if (exists(@supra_framework)) { let ban_registry_config = borrow_global(@supra_framework); @@ -139,11 +141,12 @@ module supra_framework::leader_ban_registry_config { return ( ban_registry_params.initial_elections_denied, ban_registry_params.max_elections_denied, - ban_registry_params.minimum_unbanned_proposers + ban_registry_params.minimum_unbanned_proposers, + ban_registry_params.probation_elections ) } }; - (0, 0, 0) + (0, 0, 0, 0) } /// Provide initial election denied value @@ -188,19 +191,35 @@ module supra_framework::leader_ban_registry_config { 0 } + /// Provide probation elections value + public fun get_probation_elections(): u8 acquires BanRegistryParameters, BanRegistryParametersV0 { + if (exists(@supra_framework)) { + let ban_registry_config = + borrow_global(@supra_framework); + if (ban_registry_config.version == 0) { + let ban_registry_params = + borrow_global(@supra_framework); + return ban_registry_params.probation_elections + } + }; + 0 + } + /// Decoding bytes to `BanRegistryParametersV0` using bcs fun deserialise_v0_params(bytes: vector): Option { let bcs_bytes = decode_bcs::new(bytes); let initial_elections_denied: u8 = decode_bcs::peel_u8(&mut bcs_bytes); let max_elections_denied: u32 = decode_bcs::peel_u32(&mut bcs_bytes); let minimum_unbanned_proposers: u8 = decode_bcs::peel_u8(&mut bcs_bytes); + let probation_elections: u8 = decode_bcs::peel_u8(&mut bcs_bytes); // making sure no bytes left to decode means correct parameter version if (vector::length(&decode_bcs::into_remainder_bytes(bcs_bytes)) == 0) { return option::some( BanRegistryParametersV0 { initial_elections_denied, max_elections_denied, - minimum_unbanned_proposers + minimum_unbanned_proposers, + probation_elections } ) }; @@ -212,7 +231,8 @@ module supra_framework::leader_ban_registry_config { BanRegistryParametersV0 { initial_elections_denied: 1, max_elections_denied: 5, - minimum_unbanned_proposers: 2 + minimum_unbanned_proposers: 2, + probation_elections: 1 } } @@ -220,12 +240,14 @@ module supra_framework::leader_ban_registry_config { public fun get_custom_ban_registry_params_v0( initial_elections_denied: u8, max_elections_denied: u32, - minimum_unbanned_proposers: u8 + minimum_unbanned_proposers: u8, + probation_elections: u8 ): BanRegistryParametersV0 { BanRegistryParametersV0 { initial_elections_denied, max_elections_denied, - minimum_unbanned_proposers + minimum_unbanned_proposers, + probation_elections } } diff --git a/aptos-move/framework/supra-framework/sources/leader_ban_registry.move b/aptos-move/framework/supra-framework/sources/leader_ban_registry.move index bd4e9b1fe0cfb..e6da7093170f3 100644 --- a/aptos-move/framework/supra-framework/sources/leader_ban_registry.move +++ b/aptos-move/framework/supra-framework/sources/leader_ban_registry.move @@ -42,7 +42,9 @@ module supra_framework::leader_ban_registry { /// Consecutive ban count consecutive_bans: u32, /// Validator's pool address - pool_address: address + pool_address: address, + /// Whether the validator is on probation (ban expired but still in registry) + on_probation: bool } /// Holds ban registry @@ -73,7 +75,18 @@ module supra_framework::leader_ban_registry { } #[event] - /// Emits when validator ban lifted due to ban expiry + /// Emits when validator ban lifted and probation period starts + struct ReinstatedWithProbation has drop, store { + /// Validator's pool address + pool_address: address, + /// Epoch + epoch: u64, + /// Round + round: u64 + } + + #[event] + /// Emits when validator probation period ends and is fully reinstated struct Reinstated has drop, store { /// Validator's pool address pool_address: address, @@ -137,6 +150,14 @@ module supra_framework::leader_ban_registry { committee_size * (max_elections_denied as u64) } + #[view] + /// Returns probation duration (constant, does not change based on consecutive bans) + public fun get_probation_duration(): u64 { + let probation_elections = leader_ban_registry_config::get_probation_elections(); + let committee_size = stake::get_committee_size(); + committee_size * (probation_elections as u64) + } + /// Add or update the ban registry as per block metadata public(friend) fun update_ban_registry( current_epoch: u64, @@ -189,11 +210,14 @@ module supra_framework::leader_ban_registry { } ); if (is_banned) { + // Validator is already in registry (either banned or on probation) + // Re-banning resets the ban period and increases consecutive count let bans = vector::borrow_mut(&mut ban_registry.bans, index); bans.consecutive_bans = bans.consecutive_bans + 1; - // bans.active.round_earned = latest_view.round; - // bans.active.epoch_earned = latest_view.epoch; - // bans.active.rounds_served_in_previous_epochs = 0; + bans.active.round_earned = latest_view.round; + bans.active.epoch_earned = latest_view.epoch; + bans.active.rounds_served_in_previous_epochs = 0; + bans.on_probation = false; // Reset to banned state if (features::module_event_enabled()) { event::emit( Banned { @@ -214,7 +238,8 @@ module supra_framework::leader_ban_registry { rounds_served_in_previous_epochs: 0 }, consecutive_bans: 0, - pool_address: validator_pool_address + pool_address: validator_pool_address, + on_probation: false }; vector::push_back(&mut ban_registry.bans, ban_with_address); @@ -235,32 +260,80 @@ module supra_framework::leader_ban_registry { ); } - /// Removes bans for those validator which duration is over + /// Handles ban and probation expiry: + /// - When ban expires: transitions to probation, emits ReinstatedWithProbation + /// - When probation expires: removes from registry, emits Reinstated fun reinstate_expired_bans( latest_view: &LatestView, ban_registry: &mut BanRegistry ) { - let pool_address_for_duration_over = vector::empty(); + let probation_duration = get_probation_duration(); + + // First pass: transition expired bans to probation + let pool_addresses_to_probation = vector::empty(); vector::for_each_ref( &ban_registry.bans, |v| { - if (remaining_duration(v, latest_view) == 0) { - vector::push_back( - &mut pool_address_for_duration_over, v.pool_address - ); + let v: &ValidatorBansWithAddress = v; + if (!v.on_probation && remaining_ban_duration(v, latest_view) == 0) { + vector::push_back(&mut pool_addresses_to_probation, v.pool_address); } } ); + vector::for_each_ref( - &pool_address_for_duration_over, + &pool_addresses_to_probation, |p| { - let (is_banned, index) = vector::find( + let (found, index) = vector::find( &ban_registry.bans, |v| { let v: &ValidatorBansWithAddress = v; &v.pool_address == p } ); - if (is_banned) { + if (found) { + let ban = vector::borrow_mut(&mut ban_registry.bans, index); + ban.on_probation = true; + // Reset active fields so probation duration is calculated from this point + ban.active.epoch_earned = latest_view.epoch; + ban.active.round_earned = latest_view.round; + ban.active.rounds_served_in_previous_epochs = 0; + if (features::module_event_enabled()) { + event::emit( + ReinstatedWithProbation { + epoch: latest_view.epoch, + round: latest_view.round, + pool_address: *p + } + ) + } + } + } + ); + + // Second pass: remove validators whose probation has expired + let pool_addresses_for_full_reinstatement = vector::empty(); + vector::for_each_ref( + &ban_registry.bans, + |v| { + let v: &ValidatorBansWithAddress = v; + if (v.on_probation + && remaining_probation_duration(v, latest_view, probation_duration) == 0) { + vector::push_back(&mut pool_addresses_for_full_reinstatement, v.pool_address); + } + } + ); + + vector::for_each_ref( + &pool_addresses_for_full_reinstatement, + |p| { + let (found, index) = vector::find( + &ban_registry.bans, + |v| { + let v: &ValidatorBansWithAddress = v; + &v.pool_address == p + } + ); + if (found) { vector::swap_remove(&mut ban_registry.bans, index); if (features::module_event_enabled()) { event::emit( @@ -374,8 +447,8 @@ module supra_framework::leader_ban_registry { ) } - /// Calculate the number of rounds remaining in the a given ban. - fun remaining_duration( + /// Calculate the number of rounds remaining in a given ban (not including probation). + fun remaining_ban_duration( ban: &ValidatorBansWithAddress, latest_view: &LatestView ): u64 { let initial_ban_duration = get_initial_ban_duration(); @@ -393,6 +466,22 @@ module supra_framework::leader_ban_registry { } else { 0 } } + /// Calculate the number of rounds remaining in probation. + /// Probation duration is constant and does not scale with consecutive bans. + fun remaining_probation_duration( + ban: &ValidatorBansWithAddress, latest_view: &LatestView, probation_duration: u64 + ): u64 { + let rounds_served = + if (latest_view.epoch > ban.active.epoch_earned) { + ban.active.rounds_served_in_previous_epochs + latest_view.round + } else { + latest_view.round - ban.active.round_earned + }; + if (probation_duration >= rounds_served) { + probation_duration - rounds_served + } else { 0 } + } + /// Returns true until ban registry size + minimum proposers required count less than committee size fun can_be_banned(ban_registry_len: u64): bool { let minimum_unbanned_proposers = @@ -422,4 +511,11 @@ module supra_framework::leader_ban_registry { ): u32 { validator_with_pool_addr.consecutive_bans } + + #[test_only] + public fun is_on_probation_from_vp( + validator_with_pool_addr: &ValidatorBansWithAddress + ): bool { + validator_with_pool_addr.on_probation + } } diff --git a/aptos-move/framework/supra-framework/tests/test_leader_ban_registry.move b/aptos-move/framework/supra-framework/tests/test_leader_ban_registry.move index c529c45f4aaae..f390361532a6e 100644 --- a/aptos-move/framework/supra-framework/tests/test_leader_ban_registry.move +++ b/aptos-move/framework/supra-framework/tests/test_leader_ban_registry.move @@ -198,6 +198,9 @@ module std::test_leader_ban_registry { assert!(leader_ban_registry::get_consecutive_count_from_vp(vp) == 3, 8); assert!(vector::length(&ban_registry) == 1, 9); + // At round 28, validator 1 is banned again, consecutive_bans=4 + // Ban duration = min(4 * 2^4, 20) = 20 rounds (capped at max) + // With the reset, the ban starts fresh from round 28 leader_ban_registry::update_ban_registry( 0, 12 + (4 * 4), option::some(1), vector[0] ); @@ -206,35 +209,118 @@ module std::test_leader_ban_registry { assert!(leader_ban_registry::get_consecutive_count_from_vp(vp) == 4, 8); assert!(vector::length(&ban_registry) == 1, 9); - // at 5th interval even though validator 1 is added a failed proposal index it will be removed from registry - // because max duration is set to 5 - leader_ban_registry::update_ban_registry( - 0, 12 + (4 * 5), option::some(1), vector[0] - ); + // Now test ban expiry WITHOUT re-banning the validator + // The ban was set at round 28 with duration 20, so ban expires at round 28 + 20 = 48 + // Then probation lasts for 4 rounds (probation_elections=1 * committee_size=4) + // Full reinstatement happens at round 48 + 4 = 52 + + // At round 47, the ban should still be active (1 round remaining) + leader_ban_registry::update_ban_registry(0, 47, option::some(1), vector::empty()); let ban_registry = leader_ban_registry::get_ban_registry(); - assert!(vector::length(&ban_registry) == 0, 10); + assert!(vector::length(&ban_registry) == 1, 10); + let vp = vector::borrow(&ban_registry, 0); + assert!(!leader_ban_registry::is_on_probation_from_vp(vp), 100); // Still banned, not on probation + + // At round 48, the ban expires and validator transitions to probation + leader_ban_registry::update_ban_registry(0, 48, option::some(1), vector::empty()); + let ban_registry = leader_ban_registry::get_ban_registry(); + assert!(vector::length(&ban_registry) == 1, 11); // Still in registry (on probation) + let vp = vector::borrow(&ban_registry, 0); + assert!(leader_ban_registry::is_on_probation_from_vp(vp), 101); // Now on probation + + // At round 51, probation should still be active (1 round remaining) + leader_ban_registry::update_ban_registry(0, 51, option::some(1), vector::empty()); + let ban_registry = leader_ban_registry::get_ban_registry(); + assert!(vector::length(&ban_registry) == 1, 12); + let vp = vector::borrow(&ban_registry, 0); + assert!(leader_ban_registry::is_on_probation_from_vp(vp), 102); + + // At round 52, probation expires and validator is fully reinstated + leader_ban_registry::update_ban_registry(0, 52, option::some(1), vector::empty()); + let ban_registry = leader_ban_registry::get_ban_registry(); + assert!(vector::length(&ban_registry) == 0, 13); // Fully removed from registry // Now lets try to fail 3 of them - leader_ban_registry::update_ban_registry(0, 33, option::some(3), vector[0, 1, 2]); + leader_ban_registry::update_ban_registry(0, 60, option::some(3), vector[0, 1, 2]); let ban_registry = leader_ban_registry::get_ban_registry(); // only 2 will be banned because of proposer limit of 2 - assert!(vector::length(&ban_registry) == 2, 11); + assert!(vector::length(&ban_registry) == 2, 14); + // Re-ban both validators to increase consecutive count + // Round 64: re-ban, consecutive_bans=1, duration=8, resets from round 64 leader_ban_registry::update_ban_registry( - 0, 33 + (4 * 1), option::some(2), vector[0, 1] + 0, 60 + (4 * 1), option::some(2), vector[0, 1] ); + // Round 68: re-ban, consecutive_bans=2, duration=16, resets from round 68 leader_ban_registry::update_ban_registry( - 0, 33 + (4 * 2), option::some(2), vector[0, 1] + 0, 60 + (4 * 2), option::some(2), vector[0, 1] ); + + // Now test ban expiry across epoch change WITHOUT re-banning + // Ban was set at round 68 with duration 16, so ban expires at round 68 + 16 = 84 + // Then probation lasts for 4 rounds, full reinstatement at round 84 + 4 = 88 + // First trigger epoch change leader_ban_registry::on_new_epoch(); // assuming every time this is called first before any epoch change - leader_ban_registry::update_ban_registry(1, 0, option::some(2), vector[0, 1]); - leader_ban_registry::update_ban_registry(1, (4 * 1), option::some(2), vector[0, 1]); - leader_ban_registry::update_ban_registry(1, (4 * 2), option::some(2), vector[0, 1]); - leader_ban_registry::update_ban_registry(1, (4 * 3), option::some(2), vector[0, 1]); + // In epoch 1, advance rounds without re-banning to let the ban expire naturally + // Previous epoch ended at round 68, rounds_served_in_previous_epochs will be updated + // For validators banned at epoch 0 round 68: + // After on_new_epoch: rounds_served_in_previous_epochs = 0 + (68 - 68) = 0 + // (since epoch_earned == latest_view.epoch, we use latest_view.round - round_earned) + + // At epoch 1 round 0, ban should still be active (rounds_served = 0 + 0 = 0, need 16) + leader_ban_registry::update_ban_registry(1, 0, option::some(2), vector::empty()); + let ban_registry = leader_ban_registry::get_ban_registry(); + assert!(vector::length(&ban_registry) == 2, 15); + let vp = vector::borrow(&ban_registry, 0); + assert!(!leader_ban_registry::is_on_probation_from_vp(vp), 103); + + // At epoch 1 round 15, ban should still be active (rounds_served = 0 + 15 = 15, need 16) + leader_ban_registry::update_ban_registry(1, 15, option::some(2), vector::empty()); let ban_registry = leader_ban_registry::get_ban_registry(); - // after epoch change and enough round the ban has been lifted correctly. - assert!(vector::length(&ban_registry) == 0, 12); + assert!(vector::length(&ban_registry) == 2, 16); + + // At epoch 1 round 16, ban expires and validators transition to probation + leader_ban_registry::update_ban_registry(1, 16, option::some(2), vector::empty()); + let ban_registry = leader_ban_registry::get_ban_registry(); + assert!(vector::length(&ban_registry) == 2, 17); // Still in registry (on probation) + let vp = vector::borrow(&ban_registry, 0); + assert!(leader_ban_registry::is_on_probation_from_vp(vp), 104); + + // At epoch 1 round 19, probation still active (1 round remaining) + leader_ban_registry::update_ban_registry(1, 19, option::some(2), vector::empty()); + let ban_registry = leader_ban_registry::get_ban_registry(); + assert!(vector::length(&ban_registry) == 2, 18); + + // At epoch 1 round 20, probation expires and validators are fully reinstated + leader_ban_registry::update_ban_registry(1, 20, option::some(2), vector::empty()); + let ban_registry = leader_ban_registry::get_ban_registry(); + // after epoch change and enough rounds, the ban and probation have been lifted correctly + assert!(vector::length(&ban_registry) == 0, 19); + + // Test: Re-banning during probation should increase consecutive count + // Ban validator 1 fresh + leader_ban_registry::update_ban_registry(1, 25, option::some(2), vector[0]); + let ban_registry = leader_ban_registry::get_ban_registry(); + assert!(vector::length(&ban_registry) == 1, 20); + let vp = vector::borrow(&ban_registry, 0); + assert!(leader_ban_registry::get_consecutive_count_from_vp(vp) == 0, 105); + assert!(!leader_ban_registry::is_on_probation_from_vp(vp), 106); + + // Ban duration = 4 rounds, so ban expires at round 25 + 4 = 29 + // Transition to probation at round 29 + leader_ban_registry::update_ban_registry(1, 29, option::some(2), vector::empty()); + let ban_registry = leader_ban_registry::get_ban_registry(); + let vp = vector::borrow(&ban_registry, 0); + assert!(leader_ban_registry::is_on_probation_from_vp(vp), 107); + assert!(leader_ban_registry::get_consecutive_count_from_vp(vp) == 0, 108); + + // Re-ban during probation should increase consecutive count and reset to banned state + leader_ban_registry::update_ban_registry(1, 30, option::some(2), vector[0]); + let ban_registry = leader_ban_registry::get_ban_registry(); + let vp = vector::borrow(&ban_registry, 0); + assert!(leader_ban_registry::get_consecutive_count_from_vp(vp) == 1, 109); // Consecutive count increased + assert!(!leader_ban_registry::is_on_probation_from_vp(vp), 110); // Back to banned state } } diff --git a/aptos-move/framework/supra-framework/tests/test_leader_ban_registry_config.move b/aptos-move/framework/supra-framework/tests/test_leader_ban_registry_config.move index c248a20ca9a36..80839a1aeb768 100644 --- a/aptos-move/framework/supra-framework/tests/test_leader_ban_registry_config.move +++ b/aptos-move/framework/supra-framework/tests/test_leader_ban_registry_config.move @@ -89,12 +89,14 @@ module supra_framework::test_leader_ban_registry_config { let updated_initial_e_denied = 3; let updated_max_e_denied = 8; let updated_minimum_u_proposers = 6; + let updated_probation_elections = 2; let ban_registry_param = leader_ban_registry_config::get_custom_ban_registry_params_v0( updated_initial_e_denied, updated_max_e_denied, - updated_minimum_u_proposers + updated_minimum_u_proposers, + updated_probation_elections ); let config_bytes = bcs::to_bytes(&ban_registry_param); leader_ban_registry_config::set_for_next_epoch(sender, config_bytes, 0); @@ -114,5 +116,10 @@ module supra_framework::test_leader_ban_registry_config { == updated_minimum_u_proposers, 5 ); + assert!( + leader_ban_registry_config::get_probation_elections() + == updated_probation_elections, + 6 + ); } } From ebc737f262eb273ea85c4080987b396706416545 Mon Sep 17 00:00:00 2001 From: Dhaval Purohit Date: Mon, 9 Feb 2026 20:00:07 +0530 Subject: [PATCH 13/14] added docs from comments and removed reinstate proposer --- .../configs/leader_ban_registry_config.move | 7 +- .../sources/leader_ban_registry.move | 235 ++++++++++-------- .../tests/test_leader_ban_registry.move | 194 +++++++++------ 3 files changed, 251 insertions(+), 185 deletions(-) diff --git a/aptos-move/framework/supra-framework/sources/configs/leader_ban_registry_config.move b/aptos-move/framework/supra-framework/sources/configs/leader_ban_registry_config.move index 87fe77a1beab4..0c1ac2066d544 100644 --- a/aptos-move/framework/supra-framework/sources/configs/leader_ban_registry_config.move +++ b/aptos-move/framework/supra-framework/sources/configs/leader_ban_registry_config.move @@ -41,9 +41,12 @@ module supra_framework::leader_ban_registry_config { initial_elections_denied: u8, /// Denotes max election count denied max_elections_denied: u32, - /// Denotes minimum unbanned proposer count + /// Denotes the minimum number of validators that must remain eligible for proposal. This + /// helps to preserve liveness in the presence of extended periods of network asynchrony. minimum_unbanned_proposers: u8, - /// Denotes the number of elections a validator must serve on probation after ban expires + /// Denotes the number of elections a validator must serve on probation after ban expires. + /// The ban duration compounds each time a validator is banned whilst on probation, and + /// resets to the base duration if the validator passes probation. probation_elections: u8 } diff --git a/aptos-move/framework/supra-framework/sources/leader_ban_registry.move b/aptos-move/framework/supra-framework/sources/leader_ban_registry.move index e6da7093170f3..5dcbd67db8f9d 100644 --- a/aptos-move/framework/supra-framework/sources/leader_ban_registry.move +++ b/aptos-move/framework/supra-framework/sources/leader_ban_registry.move @@ -1,4 +1,7 @@ -/// Maintains the list of banned validators and updates counters on every epoch +/// Maintains the list of banned validators and updates counters on every epoch. +/// +/// This implementation assumes that each validator is elected once every `n` consensus rounds on expectation, +/// where `n` is the number of validators in the consensus committee (i.e. the `ValidatorSet`). module supra_framework::leader_ban_registry { use std::error; use std::features; @@ -25,32 +28,37 @@ module supra_framework::leader_ban_registry { /// Latest view already initialized const ELATEST_VIEW_ALREADY_EXISTS: u64 = 3; - /// Holds metrics for banned round, epoch and round served in prev epoch + /// Information about a ban that is currently in effect for a validator. struct ActiveBan has store, drop, copy { - /// Epoch in which the ban was issued + /// The consensus epoch in which the current ban was issued (if `on_probation` is `false`) or + /// when its probation period started (if `on_probation` is `true`). epoch_earned: u64, - /// Round in which the ban was issued + /// The consensus round in which the current ban was issued (if `on_probation` is `false`) or + /// when its probation period started (if `on_probation` is `true`). round_earned: u64, /// Round count incremented on every epoch change - rounds_served_in_previous_epochs: u64 + rounds_served_in_previous_epochs: u64, + /// If `true` then the current ban period has expired and the validator is currently on probation; i.e., it is + /// eligible for election but will be banned for a longer period if it once again fails to propose a canonical + /// block when elected. If `true` then the other fields of this struct denote the consensus view in which + /// the probation period started. + on_probation: bool } /// Holds validator metrics regarding duration pool address etc - struct ValidatorBansWithAddress has store, drop, copy { - /// Holds active ban counts + struct ValidatorBans has store, drop, copy { + /// Information about the ban that is currently in effect. active: ActiveBan, - /// Consecutive ban count + /// The number of consecutive probations that this validator has failed. consecutive_bans: u32, /// Validator's pool address - pool_address: address, - /// Whether the validator is on probation (ban expired but still in registry) - on_probation: bool + pool_address: address } /// Holds ban registry struct BanRegistry has drop, store, key { /// List of validator active bans with pool address - bans: vector + bans: vector } /// Holds latest processed round and epoch @@ -70,12 +78,13 @@ module supra_framework::leader_ban_registry { epoch: u64, /// Round round: u64, - /// Consecutive bans count + /// The number of consecutive probations that this validator has failed. consecutive_bans: u32 } #[event] - /// Emits when validator ban lifted and probation period starts + /// Emitted when a validator's ban is lifted and its probation period starts. A validator that is on probation is + /// eligible for election again, but will be banned for longer if it once again fails to propose a canonical block. struct ReinstatedWithProbation has drop, store { /// Validator's pool address pool_address: address, @@ -86,7 +95,7 @@ module supra_framework::leader_ban_registry { } #[event] - /// Emits when validator probation period ends and is fully reinstated + /// Emitted when a validator's probation period ends without the validator earning a new ban. struct Reinstated has drop, store { /// Validator's pool address pool_address: address, @@ -115,7 +124,7 @@ module supra_framework::leader_ban_registry { #[view] /// Returns list of validators active ban with it's pool address - public fun get_ban_registry(): vector acquires BanRegistry { + public fun get_ban_registry(): vector acquires BanRegistry { if (!exists(@supra_framework)) { return vector::empty() }; @@ -124,7 +133,7 @@ module supra_framework::leader_ban_registry { } #[view] - /// Return initial ban duration + /// Return latest view public fun get_latest_view(): LatestView acquires LatestView { if (!exists(@supra_framework)) { return LatestView { epoch: 0, round: 0 } @@ -134,7 +143,8 @@ module supra_framework::leader_ban_registry { } #[view] - /// Return initial ban duration + /// Returns the number of consensus rounds that a validator is banned for when it fails to propose a + /// canonical block when elected as leader whilst not on probation. public fun get_initial_ban_duration(): u64 { let initial_elections_denied = leader_ban_registry_config::get_initial_elections_denied(); @@ -143,7 +153,8 @@ module supra_framework::leader_ban_registry { } #[view] - /// Returns max ban duration + /// Returns the maximum number of consensus rounds that a validator may banned for when it repeatedly fails to + /// propose a canonical block when elected as leader whilst on probation. public fun get_max_ban_duration(): u64 { let max_elections_denied = leader_ban_registry_config::get_max_elections_denied(); let committee_size = stake::get_committee_size(); @@ -151,13 +162,61 @@ module supra_framework::leader_ban_registry { } #[view] - /// Returns probation duration (constant, does not change based on consecutive bans) + /// Returns the number of consensus rounds that a validator is considered to be on probation for after having + /// served its most recent ban. public fun get_probation_duration(): u64 { let probation_elections = leader_ban_registry_config::get_probation_elections(); let committee_size = stake::get_committee_size(); committee_size * (probation_elections as u64) } + #[view] + /// Returns the number of consensus rounds remaining in the ban for the validator with the given + /// pool address. Returns 0 if the validator is not banned (including if it is on probation). + public fun get_remaining_ban_duration(pool_address: address): u64 acquires BanRegistry, LatestView { + if (!exists(@supra_framework) || !exists(@supra_framework)) { + return 0 + }; + let ban_registry = borrow_global(@supra_framework); + let latest_view = borrow_global(@supra_framework); + let (found, index) = vector::find( + &ban_registry.bans, + |v| { + let v: &ValidatorBans = v; + v.pool_address == pool_address && !v.active.on_probation + } + ); + if (found) { + remaining_ban_duration(vector::borrow(&ban_registry.bans, index), latest_view) + } else { + 0 + } + } + + #[view] + /// Returns the number of consensus rounds remaining in the probation period for the validator + /// with the given pool address. Returns 0 if the validator is not on probation. + public fun get_remaining_probation_duration(pool_address: address): u64 acquires BanRegistry, LatestView { + if (!exists(@supra_framework) || !exists(@supra_framework)) { + return 0 + }; + let ban_registry = borrow_global(@supra_framework); + let latest_view = borrow_global(@supra_framework); + let (found, index) = vector::find( + &ban_registry.bans, + |v| { + let v: &ValidatorBans = v; + v.pool_address == pool_address && v.active.on_probation + } + ); + if (found) { + let probation_dur = get_probation_duration(); + remaining_probation_duration(vector::borrow(&ban_registry.bans, index), latest_view, probation_dur) + } else { + 0 + } + } + /// Add or update the ban registry as per block metadata public(friend) fun update_ban_registry( current_epoch: u64, @@ -177,12 +236,6 @@ module supra_framework::leader_ban_registry { // remove expired bans reinstate_expired_bans(latest_view, ban_registry); - - // unban the proposer - if (std::option::is_some(&proposer_index)) { - let extracted_proposer_index = *std::option::borrow(&mut proposer_index); - reinstate_proposer(latest_view, extracted_proposer_index, ban_registry); - }; } /// Adds failed proposer indices to ban registry @@ -199,25 +252,30 @@ module supra_framework::leader_ban_registry { |failed_validator_index| { let validator_pool_address_opt = stake::get_pool_address_from_index(failed_validator_index); + if (option::is_some(&validator_pool_address_opt)) { let validator_pool_address = option::extract(&mut validator_pool_address_opt); let (is_banned, index) = vector::find( &ban_registry.bans, |v| { - let v: &ValidatorBansWithAddress = v; + let v: &ValidatorBans = v; validator_pool_address == v.pool_address } ); if (is_banned) { - // Validator is already in registry (either banned or on probation) - // Re-banning resets the ban period and increases consecutive count + // Validator is already in registry (either banned or on probation). + // Re-banning resets the ban period and increases consecutive count. + // If the consensus code is implemented correctly then the validator should + // not be re-banned whilst serving a ban as it should not be eligible for election + // when banned (i.e. this branch should only be taken when a validator is on probation). let bans = vector::borrow_mut(&mut ban_registry.bans, index); bans.consecutive_bans = bans.consecutive_bans + 1; bans.active.round_earned = latest_view.round; bans.active.epoch_earned = latest_view.epoch; bans.active.rounds_served_in_previous_epochs = 0; - bans.on_probation = false; // Reset to banned state + bans.active.on_probation = false; // Reset to banned state + if (features::module_event_enabled()) { event::emit( Banned { @@ -231,15 +289,15 @@ module supra_framework::leader_ban_registry { } else { let ban_registry_len = vector::length(&ban_registry.bans); if (can_be_banned(ban_registry_len)) { - let ban_with_address = ValidatorBansWithAddress { + let ban_with_address = ValidatorBans { active: ActiveBan { epoch_earned: latest_view.epoch, round_earned: latest_view.round, - rounds_served_in_previous_epochs: 0 + rounds_served_in_previous_epochs: 0, + on_probation: false }, consecutive_bans: 0, - pool_address: validator_pool_address, - on_probation: false + pool_address: validator_pool_address }; vector::push_back(&mut ban_registry.bans, ban_with_address); @@ -273,8 +331,8 @@ module supra_framework::leader_ban_registry { vector::for_each_ref( &ban_registry.bans, |v| { - let v: &ValidatorBansWithAddress = v; - if (!v.on_probation && remaining_ban_duration(v, latest_view) == 0) { + let v: &ValidatorBans = v; + if (!v.active.on_probation && remaining_ban_duration(v, latest_view) == 0) { vector::push_back(&mut pool_addresses_to_probation, v.pool_address); } } @@ -286,17 +344,18 @@ module supra_framework::leader_ban_registry { let (found, index) = vector::find( &ban_registry.bans, |v| { - let v: &ValidatorBansWithAddress = v; + let v: &ValidatorBans = v; &v.pool_address == p } ); if (found) { let ban = vector::borrow_mut(&mut ban_registry.bans, index); - ban.on_probation = true; + ban.active.on_probation = true; // Reset active fields so probation duration is calculated from this point ban.active.epoch_earned = latest_view.epoch; ban.active.round_earned = latest_view.round; ban.active.rounds_served_in_previous_epochs = 0; + if (features::module_event_enabled()) { event::emit( ReinstatedWithProbation { @@ -315,8 +374,8 @@ module supra_framework::leader_ban_registry { vector::for_each_ref( &ban_registry.bans, |v| { - let v: &ValidatorBansWithAddress = v; - if (v.on_probation + let v: &ValidatorBans = v; + if (v.active.on_probation && remaining_probation_duration(v, latest_view, probation_duration) == 0) { vector::push_back(&mut pool_addresses_for_full_reinstatement, v.pool_address); } @@ -329,12 +388,13 @@ module supra_framework::leader_ban_registry { let (found, index) = vector::find( &ban_registry.bans, |v| { - let v: &ValidatorBansWithAddress = v; + let v: &ValidatorBans = v; &v.pool_address == p } ); if (found) { vector::swap_remove(&mut ban_registry.bans, index); + if (features::module_event_enabled()) { event::emit( Reinstated { @@ -349,40 +409,12 @@ module supra_framework::leader_ban_registry { ); } - /// Remvoing an entry of validator from registry if found at proposer index - fun reinstate_proposer( - latest_view: &LatestView, proposer_index: u64, ban_registry: &mut BanRegistry - ) { - let validator_pool_address_opt = - stake::get_pool_address_from_index(proposer_index); - if (option::is_some(&validator_pool_address_opt)) { - let validator_pool_address = option::extract( - &mut validator_pool_address_opt - ); - let (is_banned, index) = vector::find( - &ban_registry.bans, - |v| { - let v: &ValidatorBansWithAddress = v; - validator_pool_address == v.pool_address - } - ); - if (is_banned) { - vector::swap_remove(&mut ban_registry.bans, index); - if (features::module_event_enabled()) { - event::emit( - Reinstated { - round: latest_view.round, - epoch: latest_view.epoch, - pool_address: validator_pool_address - } - ) - } - }; - }; - } - - /// Update counts on every epoch - /// Only run after committee has been updated from reconfigure + /// Increments the total number of consensus rounds served by each banned validator and removes + /// registry entries for validators that have left the validator set. + /// + /// The number of rounds in each epoch may vary due to network asynchrony, so we must record the + /// number of rounds served in previous epochs to be able to ensure that a banned validator serves its + /// full ban period when its ban span multiple epochs. public(friend) fun on_new_epoch() acquires BanRegistry, LatestView { if (!exists(@supra_framework)) { return }; if (!exists(@supra_framework)) { return }; @@ -396,17 +428,16 @@ module supra_framework::leader_ban_registry { vector::for_each_mut( &mut ban_registry.bans, |v| { - let v: &mut ValidatorBansWithAddress = v; - if (has_started(&v.active, latest_view)) { + let v: &mut ValidatorBans = v; + if (vector::contains(&new_committee_pool_addresses, &v.pool_address)) { if (latest_view.epoch > v.active.epoch_earned) { v.active.rounds_served_in_previous_epochs = v.active.rounds_served_in_previous_epochs + latest_view.round; - } else { - v.active.rounds_served_in_previous_epochs = v.active.rounds_served_in_previous_epochs - + latest_view.round - v.active.round_earned; - } - }; - if (!vector::contains(&new_committee_pool_addresses, &v.pool_address)) { + } else if (latest_view.epoch == v.active.epoch_earned && latest_view.round > v.active.round_earned) { + v.active.rounds_served_in_previous_epochs = latest_view.round - v.active.round_earned; + }; + // else: The ban hasn't started yet. + } else { vector::push_back(&mut retired_validators, v.pool_address) } } @@ -418,12 +449,13 @@ module supra_framework::leader_ban_registry { let (is_banned, index) = vector::find( &ban_registry.bans, |v| { - let v: &ValidatorBansWithAddress = v; + let v: &ValidatorBans = v; &v.pool_address == p } ); if (is_banned) { vector::swap_remove(&mut ban_registry.bans, index); + if (features::module_event_enabled()) { event::emit( Reinstated { @@ -438,18 +470,9 @@ module supra_framework::leader_ban_registry { ); } - /// Checks if ban has already started - fun has_started(active_ban: &ActiveBan, latest_view: &LatestView): bool { - active_ban.epoch_earned < latest_view.epoch - || ( - active_ban.epoch_earned == latest_view.epoch - && active_ban.round_earned < latest_view.round - ) - } - /// Calculate the number of rounds remaining in a given ban (not including probation). fun remaining_ban_duration( - ban: &ValidatorBansWithAddress, latest_view: &LatestView + ban: &ValidatorBans, latest_view: &LatestView ): u64 { let initial_ban_duration = get_initial_ban_duration(); let max_ban_duration = get_max_ban_duration(); @@ -458,10 +481,13 @@ module supra_framework::leader_ban_registry { let rounds_served = if (latest_view.epoch > ban.active.epoch_earned) { ban.active.rounds_served_in_previous_epochs + latest_view.round - } else { + } else if (latest_view.epoch == ban.active.epoch_earned && latest_view.round > ban.active.round_earned) { latest_view.round - ban.active.round_earned + } else { + // The ban hasn't started yet. + 0 }; - if (duration >= rounds_served) { + if (duration > rounds_served) { duration - rounds_served } else { 0 } } @@ -469,15 +495,18 @@ module supra_framework::leader_ban_registry { /// Calculate the number of rounds remaining in probation. /// Probation duration is constant and does not scale with consecutive bans. fun remaining_probation_duration( - ban: &ValidatorBansWithAddress, latest_view: &LatestView, probation_duration: u64 + ban: &ValidatorBans, latest_view: &LatestView, probation_duration: u64 ): u64 { let rounds_served = if (latest_view.epoch > ban.active.epoch_earned) { ban.active.rounds_served_in_previous_epochs + latest_view.round - } else { + } else if (latest_view.epoch == ban.active.epoch_earned && latest_view.round > ban.active.round_earned) { latest_view.round - ban.active.round_earned + } else { + // The ban hasn't started yet. + 0 }; - if (probation_duration >= rounds_served) { + if (probation_duration > rounds_served) { probation_duration - rounds_served } else { 0 } } @@ -500,22 +529,22 @@ module supra_framework::leader_ban_registry { #[test_only] public fun get_pool_address_from_vp( - validator_with_pool_addr: &ValidatorBansWithAddress + validator_with_pool_addr: &ValidatorBans ): address { validator_with_pool_addr.pool_address } #[test_only] public fun get_consecutive_count_from_vp( - validator_with_pool_addr: &ValidatorBansWithAddress + validator_with_pool_addr: &ValidatorBans ): u32 { validator_with_pool_addr.consecutive_bans } #[test_only] public fun is_on_probation_from_vp( - validator_with_pool_addr: &ValidatorBansWithAddress + validator_with_pool_addr: &ValidatorBans ): bool { - validator_with_pool_addr.on_probation + validator_with_pool_addr.active.on_probation } } diff --git a/aptos-move/framework/supra-framework/tests/test_leader_ban_registry.move b/aptos-move/framework/supra-framework/tests/test_leader_ban_registry.move index f390361532a6e..122871d34aa96 100644 --- a/aptos-move/framework/supra-framework/tests/test_leader_ban_registry.move +++ b/aptos-move/framework/supra-framework/tests/test_leader_ban_registry.move @@ -136,12 +136,15 @@ module std::test_leader_ban_registry { validator_4 ); - // update ban registry + // ===== Part 1: Basic ban -> probation -> reinstatement ===== + // Config: initial_ban_duration=4, probation_duration=4, max_ban_duration=20, committee_size=4 + + // Round 0: No failures, registry empty leader_ban_registry::update_ban_registry(0, 0, option::some(0), vector::empty()); let ban_registry = leader_ban_registry::get_ban_registry(); assert!(vector::length(&ban_registry) == 0, 1); - // Let's say for round 1 validator 2 failed to propose + // Round 1: Validator 2 (index 1) fails to propose -> banned leader_ban_registry::update_ban_registry(0, 1, option::some(2), vector[1]); let ban_registry = leader_ban_registry::get_ban_registry(); assert!(vector::length(&ban_registry) == 1, 2); @@ -153,174 +156,205 @@ module std::test_leader_ban_registry { 3 ); - // Let's say for round 5 validator 2 failed to propose again - leader_ban_registry::update_ban_registry(0, 5, option::some(2), vector[1]); + // Round 4: Ban still active (duration=4, rounds_served=3, remaining=1) + leader_ban_registry::update_ban_registry(0, 4, option::some(2), vector::empty()); let ban_registry = leader_ban_registry::get_ban_registry(); + assert!(vector::length(&ban_registry) == 1, 4); let vp = vector::borrow(&ban_registry, 0); - assert!(leader_ban_registry::get_consecutive_count_from_vp(vp) == 1, 4); + assert!(!leader_ban_registry::is_on_probation_from_vp(vp), 100); - // If on round 9 the validator 2 is able to propose then it will be removed from registry - leader_ban_registry::update_ban_registry(0, 9, option::some(1), vector[]); + // Round 5: Ban expires (rounds_served=4), transitions to probation + leader_ban_registry::update_ban_registry(0, 5, option::some(2), vector::empty()); let ban_registry = leader_ban_registry::get_ban_registry(); - assert!(vector::length(&ban_registry) == 0, 5); + assert!(vector::length(&ban_registry) == 1, 5); // Still in registry (on probation) + let vp = vector::borrow(&ban_registry, 0); + assert!(leader_ban_registry::is_on_probation_from_vp(vp), 101); + + // Round 9: Probation expires (rounds_served=4), fully reinstated + leader_ban_registry::update_ban_registry(0, 9, option::some(2), vector::empty()); + let ban_registry = leader_ban_registry::get_ban_registry(); + assert!(vector::length(&ban_registry) == 0, 6); + + // ===== Part 2: Consecutive bans via re-ban during probation ===== - // Now let's say validator 1 is unable to propose + // Round 12: Validator 1 (index 0) fails -> banned, consecutive=0, duration=4 leader_ban_registry::update_ban_registry(0, 12, option::some(1), vector[0]); let ban_registry = leader_ban_registry::get_ban_registry(); let vp = vector::borrow(&ban_registry, 0); assert!( signer::address_of(validator_1) == leader_ban_registry::get_pool_address_from_vp(vp), - 6 + 7 ); + assert!(leader_ban_registry::get_consecutive_count_from_vp(vp) == 0, 8); - leader_ban_registry::update_ban_registry( - 0, 12 + (4 * 1), option::some(1), vector[0] - ); + // Round 16: Ban expires (duration=4) -> probation + leader_ban_registry::update_ban_registry(0, 16, option::some(1), vector::empty()); let ban_registry = leader_ban_registry::get_ban_registry(); let vp = vector::borrow(&ban_registry, 0); - assert!(leader_ban_registry::get_consecutive_count_from_vp(vp) == 1, 8); - assert!(vector::length(&ban_registry) == 1, 9); + assert!(leader_ban_registry::is_on_probation_from_vp(vp), 102); - leader_ban_registry::update_ban_registry( - 0, 12 + (4 * 2), option::some(1), vector[0] - ); + // Round 17: Re-ban during probation -> consecutive=1, duration=8 + leader_ban_registry::update_ban_registry(0, 17, option::some(1), vector[0]); let ban_registry = leader_ban_registry::get_ban_registry(); let vp = vector::borrow(&ban_registry, 0); - assert!(leader_ban_registry::get_consecutive_count_from_vp(vp) == 2, 8); - assert!(vector::length(&ban_registry) == 1, 9); + assert!(leader_ban_registry::get_consecutive_count_from_vp(vp) == 1, 9); + assert!(!leader_ban_registry::is_on_probation_from_vp(vp), 103); + assert!(vector::length(&ban_registry) == 1, 10); - leader_ban_registry::update_ban_registry( - 0, 12 + (4 * 3), option::some(1), vector[0] - ); + // Round 25: Ban expires (duration=8) -> probation + leader_ban_registry::update_ban_registry(0, 25, option::some(1), vector::empty()); let ban_registry = leader_ban_registry::get_ban_registry(); let vp = vector::borrow(&ban_registry, 0); - assert!(leader_ban_registry::get_consecutive_count_from_vp(vp) == 3, 8); - assert!(vector::length(&ban_registry) == 1, 9); + assert!(leader_ban_registry::is_on_probation_from_vp(vp), 104); - // At round 28, validator 1 is banned again, consecutive_bans=4 - // Ban duration = min(4 * 2^4, 20) = 20 rounds (capped at max) - // With the reset, the ban starts fresh from round 28 - leader_ban_registry::update_ban_registry( - 0, 12 + (4 * 4), option::some(1), vector[0] - ); + // Round 26: Re-ban during probation -> consecutive=2, duration=16 + leader_ban_registry::update_ban_registry(0, 26, option::some(1), vector[0]); let ban_registry = leader_ban_registry::get_ban_registry(); let vp = vector::borrow(&ban_registry, 0); - assert!(leader_ban_registry::get_consecutive_count_from_vp(vp) == 4, 8); - assert!(vector::length(&ban_registry) == 1, 9); + assert!(leader_ban_registry::get_consecutive_count_from_vp(vp) == 2, 11); + assert!(!leader_ban_registry::is_on_probation_from_vp(vp), 105); + assert!(vector::length(&ban_registry) == 1, 12); - // Now test ban expiry WITHOUT re-banning the validator - // The ban was set at round 28 with duration 20, so ban expires at round 28 + 20 = 48 - // Then probation lasts for 4 rounds (probation_elections=1 * committee_size=4) - // Full reinstatement happens at round 48 + 4 = 52 + // Round 42: Ban expires (duration=16) -> probation + leader_ban_registry::update_ban_registry(0, 42, option::some(1), vector::empty()); + let ban_registry = leader_ban_registry::get_ban_registry(); + let vp = vector::borrow(&ban_registry, 0); + assert!(leader_ban_registry::is_on_probation_from_vp(vp), 106); - // At round 47, the ban should still be active (1 round remaining) - leader_ban_registry::update_ban_registry(0, 47, option::some(1), vector::empty()); + // Round 43: Re-ban during probation -> consecutive=3, duration=min(32,20)=20 + leader_ban_registry::update_ban_registry(0, 43, option::some(1), vector[0]); let ban_registry = leader_ban_registry::get_ban_registry(); - assert!(vector::length(&ban_registry) == 1, 10); let vp = vector::borrow(&ban_registry, 0); - assert!(!leader_ban_registry::is_on_probation_from_vp(vp), 100); // Still banned, not on probation + assert!(leader_ban_registry::get_consecutive_count_from_vp(vp) == 3, 13); + assert!(!leader_ban_registry::is_on_probation_from_vp(vp), 107); + assert!(vector::length(&ban_registry) == 1, 14); - // At round 48, the ban expires and validator transitions to probation - leader_ban_registry::update_ban_registry(0, 48, option::some(1), vector::empty()); + // Round 63: Ban expires (duration=20) -> probation + leader_ban_registry::update_ban_registry(0, 63, option::some(1), vector::empty()); let ban_registry = leader_ban_registry::get_ban_registry(); - assert!(vector::length(&ban_registry) == 1, 11); // Still in registry (on probation) let vp = vector::borrow(&ban_registry, 0); - assert!(leader_ban_registry::is_on_probation_from_vp(vp), 101); // Now on probation + assert!(leader_ban_registry::is_on_probation_from_vp(vp), 108); - // At round 51, probation should still be active (1 round remaining) - leader_ban_registry::update_ban_registry(0, 51, option::some(1), vector::empty()); + // Round 64: Re-ban during probation -> consecutive=4, duration=min(64,20)=20 + leader_ban_registry::update_ban_registry(0, 64, option::some(1), vector[0]); let ban_registry = leader_ban_registry::get_ban_registry(); - assert!(vector::length(&ban_registry) == 1, 12); let vp = vector::borrow(&ban_registry, 0); - assert!(leader_ban_registry::is_on_probation_from_vp(vp), 102); + assert!(leader_ban_registry::get_consecutive_count_from_vp(vp) == 4, 15); + assert!(!leader_ban_registry::is_on_probation_from_vp(vp), 109); + assert!(vector::length(&ban_registry) == 1, 16); - // At round 52, probation expires and validator is fully reinstated - leader_ban_registry::update_ban_registry(0, 52, option::some(1), vector::empty()); + // ===== Part 3: Natural expiry of max-duration ban ===== + // Ban at round 64 with consecutive=4, duration=20 + // Ban expires at round 84, probation starts at 84, probation expires at 88 + + // At round 83, the ban should still be active (1 round remaining) + leader_ban_registry::update_ban_registry(0, 83, option::some(1), vector::empty()); let ban_registry = leader_ban_registry::get_ban_registry(); - assert!(vector::length(&ban_registry) == 0, 13); // Fully removed from registry + assert!(vector::length(&ban_registry) == 1, 17); + let vp = vector::borrow(&ban_registry, 0); + assert!(!leader_ban_registry::is_on_probation_from_vp(vp), 110); // Still banned + // At round 84, the ban expires and validator transitions to probation + leader_ban_registry::update_ban_registry(0, 84, option::some(1), vector::empty()); + let ban_registry = leader_ban_registry::get_ban_registry(); + assert!(vector::length(&ban_registry) == 1, 18); // Still in registry (on probation) + let vp = vector::borrow(&ban_registry, 0); + assert!(leader_ban_registry::is_on_probation_from_vp(vp), 111); // Now on probation + + // At round 87, probation should still be active (1 round remaining) + leader_ban_registry::update_ban_registry(0, 87, option::some(1), vector::empty()); + let ban_registry = leader_ban_registry::get_ban_registry(); + assert!(vector::length(&ban_registry) == 1, 19); + let vp = vector::borrow(&ban_registry, 0); + assert!(leader_ban_registry::is_on_probation_from_vp(vp), 112); + + // At round 88, probation expires and validator is fully reinstated + leader_ban_registry::update_ban_registry(0, 88, option::some(1), vector::empty()); + let ban_registry = leader_ban_registry::get_ban_registry(); + assert!(vector::length(&ban_registry) == 0, 20); // Fully removed from registry + + // ===== Part 4: Multi-validator banning with proposer limit ===== // Now lets try to fail 3 of them - leader_ban_registry::update_ban_registry(0, 60, option::some(3), vector[0, 1, 2]); + leader_ban_registry::update_ban_registry(0, 90, option::some(3), vector[0, 1, 2]); let ban_registry = leader_ban_registry::get_ban_registry(); // only 2 will be banned because of proposer limit of 2 - assert!(vector::length(&ban_registry) == 2, 14); + assert!(vector::length(&ban_registry) == 2, 21); // Re-ban both validators to increase consecutive count - // Round 64: re-ban, consecutive_bans=1, duration=8, resets from round 64 + // Round 94: re-ban, consecutive_bans=1, duration=8, resets from round 94 leader_ban_registry::update_ban_registry( - 0, 60 + (4 * 1), option::some(2), vector[0, 1] + 0, 90 + (4 * 1), option::some(2), vector[0, 1] ); - // Round 68: re-ban, consecutive_bans=2, duration=16, resets from round 68 + // Round 98: re-ban, consecutive_bans=2, duration=16, resets from round 98 leader_ban_registry::update_ban_registry( - 0, 60 + (4 * 2), option::some(2), vector[0, 1] + 0, 90 + (4 * 2), option::some(2), vector[0, 1] ); + // ===== Part 5: Cross-epoch ban management ===== // Now test ban expiry across epoch change WITHOUT re-banning - // Ban was set at round 68 with duration 16, so ban expires at round 68 + 16 = 84 - // Then probation lasts for 4 rounds, full reinstatement at round 84 + 4 = 88 + // Ban was set at round 98 with duration 16 // First trigger epoch change - leader_ban_registry::on_new_epoch(); // assuming every time this is called first before any epoch change + leader_ban_registry::on_new_epoch(); // In epoch 1, advance rounds without re-banning to let the ban expire naturally - // Previous epoch ended at round 68, rounds_served_in_previous_epochs will be updated - // For validators banned at epoch 0 round 68: - // After on_new_epoch: rounds_served_in_previous_epochs = 0 + (68 - 68) = 0 - // (since epoch_earned == latest_view.epoch, we use latest_view.round - round_earned) + // For validators banned at epoch 0 round 98: + // After on_new_epoch: rounds_served_in_previous_epochs = 0 + // (since epoch_earned == latest_view.epoch and round_earned == latest_view.round) // At epoch 1 round 0, ban should still be active (rounds_served = 0 + 0 = 0, need 16) leader_ban_registry::update_ban_registry(1, 0, option::some(2), vector::empty()); let ban_registry = leader_ban_registry::get_ban_registry(); - assert!(vector::length(&ban_registry) == 2, 15); + assert!(vector::length(&ban_registry) == 2, 22); let vp = vector::borrow(&ban_registry, 0); - assert!(!leader_ban_registry::is_on_probation_from_vp(vp), 103); + assert!(!leader_ban_registry::is_on_probation_from_vp(vp), 113); // At epoch 1 round 15, ban should still be active (rounds_served = 0 + 15 = 15, need 16) leader_ban_registry::update_ban_registry(1, 15, option::some(2), vector::empty()); let ban_registry = leader_ban_registry::get_ban_registry(); - assert!(vector::length(&ban_registry) == 2, 16); + assert!(vector::length(&ban_registry) == 2, 23); // At epoch 1 round 16, ban expires and validators transition to probation leader_ban_registry::update_ban_registry(1, 16, option::some(2), vector::empty()); let ban_registry = leader_ban_registry::get_ban_registry(); - assert!(vector::length(&ban_registry) == 2, 17); // Still in registry (on probation) + assert!(vector::length(&ban_registry) == 2, 24); // Still in registry (on probation) let vp = vector::borrow(&ban_registry, 0); - assert!(leader_ban_registry::is_on_probation_from_vp(vp), 104); + assert!(leader_ban_registry::is_on_probation_from_vp(vp), 114); // At epoch 1 round 19, probation still active (1 round remaining) leader_ban_registry::update_ban_registry(1, 19, option::some(2), vector::empty()); let ban_registry = leader_ban_registry::get_ban_registry(); - assert!(vector::length(&ban_registry) == 2, 18); + assert!(vector::length(&ban_registry) == 2, 25); // At epoch 1 round 20, probation expires and validators are fully reinstated leader_ban_registry::update_ban_registry(1, 20, option::some(2), vector::empty()); let ban_registry = leader_ban_registry::get_ban_registry(); - // after epoch change and enough rounds, the ban and probation have been lifted correctly - assert!(vector::length(&ban_registry) == 0, 19); + assert!(vector::length(&ban_registry) == 0, 26); + // ===== Part 6: Re-ban during probation ===== // Test: Re-banning during probation should increase consecutive count // Ban validator 1 fresh leader_ban_registry::update_ban_registry(1, 25, option::some(2), vector[0]); let ban_registry = leader_ban_registry::get_ban_registry(); - assert!(vector::length(&ban_registry) == 1, 20); + assert!(vector::length(&ban_registry) == 1, 27); let vp = vector::borrow(&ban_registry, 0); - assert!(leader_ban_registry::get_consecutive_count_from_vp(vp) == 0, 105); - assert!(!leader_ban_registry::is_on_probation_from_vp(vp), 106); + assert!(leader_ban_registry::get_consecutive_count_from_vp(vp) == 0, 115); + assert!(!leader_ban_registry::is_on_probation_from_vp(vp), 116); // Ban duration = 4 rounds, so ban expires at round 25 + 4 = 29 // Transition to probation at round 29 leader_ban_registry::update_ban_registry(1, 29, option::some(2), vector::empty()); let ban_registry = leader_ban_registry::get_ban_registry(); let vp = vector::borrow(&ban_registry, 0); - assert!(leader_ban_registry::is_on_probation_from_vp(vp), 107); - assert!(leader_ban_registry::get_consecutive_count_from_vp(vp) == 0, 108); + assert!(leader_ban_registry::is_on_probation_from_vp(vp), 117); + assert!(leader_ban_registry::get_consecutive_count_from_vp(vp) == 0, 118); // Re-ban during probation should increase consecutive count and reset to banned state leader_ban_registry::update_ban_registry(1, 30, option::some(2), vector[0]); let ban_registry = leader_ban_registry::get_ban_registry(); let vp = vector::borrow(&ban_registry, 0); - assert!(leader_ban_registry::get_consecutive_count_from_vp(vp) == 1, 109); // Consecutive count increased - assert!(!leader_ban_registry::is_on_probation_from_vp(vp), 110); // Back to banned state + assert!(leader_ban_registry::get_consecutive_count_from_vp(vp) == 1, 119); // Consecutive count increased + assert!(!leader_ban_registry::is_on_probation_from_vp(vp), 120); // Back to banned state } } From 13112421a1988e632977a5f99eea4ed3dbf90257 Mon Sep 17 00:00:00 2001 From: Dhaval Purohit Date: Tue, 10 Feb 2026 11:51:03 +0530 Subject: [PATCH 14/14] updated the flow for expiration of bans --- .../sources/leader_ban_registry.move | 114 ++++++++++++------ 1 file changed, 75 insertions(+), 39 deletions(-) diff --git a/aptos-move/framework/supra-framework/sources/leader_ban_registry.move b/aptos-move/framework/supra-framework/sources/leader_ban_registry.move index 5dcbd67db8f9d..a04dcb4a46a8d 100644 --- a/aptos-move/framework/supra-framework/sources/leader_ban_registry.move +++ b/aptos-move/framework/supra-framework/sources/leader_ban_registry.move @@ -319,27 +319,34 @@ module supra_framework::leader_ban_registry { } /// Handles ban and probation expiry: - /// - When ban expires: transitions to probation, emits ReinstatedWithProbation /// - When probation expires: removes from registry, emits Reinstated + /// - When ban expires and probation_duration > 0: transitions to probation, emits ReinstatedWithProbation + /// - When ban expires and probation_duration == 0: removes from registry, emits Reinstated fun reinstate_expired_bans( latest_view: &LatestView, ban_registry: &mut BanRegistry ) { let probation_duration = get_probation_duration(); - // First pass: transition expired bans to probation - let pool_addresses_to_probation = vector::empty(); + // First pass: remove validators whose probation has expired. + // Done before ban-to-probation transitions to avoid iterating just-transitioned validators. + // Always runs (even when probation_duration == 0) to clean up validators that were already + // on probation before a config change set probation_duration to 0. + let pool_addresses_for_full_reinstatement = vector::empty(); vector::for_each_ref( &ban_registry.bans, |v| { let v: &ValidatorBans = v; - if (!v.active.on_probation && remaining_ban_duration(v, latest_view) == 0) { - vector::push_back(&mut pool_addresses_to_probation, v.pool_address); + if (v.active.on_probation + && remaining_probation_duration(v, latest_view, probation_duration) == 0) { + vector::push_back( + &mut pool_addresses_for_full_reinstatement, v.pool_address + ); } } ); vector::for_each_ref( - &pool_addresses_to_probation, + &pool_addresses_for_full_reinstatement, |p| { let (found, index) = vector::find( &ban_registry.bans, @@ -349,16 +356,11 @@ module supra_framework::leader_ban_registry { } ); if (found) { - let ban = vector::borrow_mut(&mut ban_registry.bans, index); - ban.active.on_probation = true; - // Reset active fields so probation duration is calculated from this point - ban.active.epoch_earned = latest_view.epoch; - ban.active.round_earned = latest_view.round; - ban.active.rounds_served_in_previous_epochs = 0; + vector::swap_remove(&mut ban_registry.bans, index); if (features::module_event_enabled()) { event::emit( - ReinstatedWithProbation { + Reinstated { epoch: latest_view.epoch, round: latest_view.round, pool_address: *p @@ -369,44 +371,78 @@ module supra_framework::leader_ban_registry { } ); - // Second pass: remove validators whose probation has expired - let pool_addresses_for_full_reinstatement = vector::empty(); + // Second pass: handle expired bans + let pool_addresses_with_expired_bans = vector::empty(); vector::for_each_ref( &ban_registry.bans, |v| { let v: &ValidatorBans = v; - if (v.active.on_probation - && remaining_probation_duration(v, latest_view, probation_duration) == 0) { - vector::push_back(&mut pool_addresses_for_full_reinstatement, v.pool_address); + if (!v.active.on_probation && remaining_ban_duration(v, latest_view) == 0) { + vector::push_back(&mut pool_addresses_with_expired_bans, v.pool_address); } } ); - vector::for_each_ref( - &pool_addresses_for_full_reinstatement, - |p| { - let (found, index) = vector::find( - &ban_registry.bans, - |v| { - let v: &ValidatorBans = v; - &v.pool_address == p + if (probation_duration > 0) { + // Transition expired bans to probation + vector::for_each_ref( + &pool_addresses_with_expired_bans, + |p| { + let (found, index) = vector::find( + &ban_registry.bans, + |v| { + let v: &ValidatorBans = v; + &v.pool_address == p + } + ); + if (found) { + let ban = vector::borrow_mut(&mut ban_registry.bans, index); + ban.active.on_probation = true; + // Reset active fields so probation duration is calculated from this point + ban.active.epoch_earned = latest_view.epoch; + ban.active.round_earned = latest_view.round; + ban.active.rounds_served_in_previous_epochs = 0; + + if (features::module_event_enabled()) { + event::emit( + ReinstatedWithProbation { + epoch: latest_view.epoch, + round: latest_view.round, + pool_address: *p + } + ) + } } - ); - if (found) { - vector::swap_remove(&mut ban_registry.bans, index); + } + ); + } else { + // No probation period - directly remove validators whose ban expired + vector::for_each_ref( + &pool_addresses_with_expired_bans, + |p| { + let (found, index) = vector::find( + &ban_registry.bans, + |v| { + let v: &ValidatorBans = v; + &v.pool_address == p + } + ); + if (found) { + vector::swap_remove(&mut ban_registry.bans, index); - if (features::module_event_enabled()) { - event::emit( - Reinstated { - epoch: latest_view.epoch, - round: latest_view.round, - pool_address: *p - } - ) + if (features::module_event_enabled()) { + event::emit( + Reinstated { + epoch: latest_view.epoch, + round: latest_view.round, + pool_address: *p + } + ) + } } } - } - ); + ); + }; } /// Increments the total number of consensus rounds served by each banned validator and removes