Skip to content

Commit

Permalink
feat(nns): Neurons created with minimum dissolve delay (#1982)
Browse files Browse the repository at this point in the history
Created neurons should have a minimal dissolve delay. This sets the
minimum dissolve delay to 1 week and puts a ceiling on neuron creation.
  • Loading branch information
max-dfinity authored Oct 18, 2024
1 parent 00a3fb9 commit 4bed17b
Show file tree
Hide file tree
Showing 7 changed files with 405 additions and 128 deletions.
95 changes: 87 additions & 8 deletions rs/nns/governance/src/governance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,9 @@ pub const EXECUTE_NNS_FUNCTION_PAYLOAD_LISTING_BYTES_MAX: usize = 1000;
// 10 KB
pub const PROPOSAL_MOTION_TEXT_BYTES_MAX: usize = 10000;

// The minimum neuron dissolve delay (set when a neuron is first claimed)
pub const INITIAL_NEURON_DISSOLVE_DELAY: u64 = 7 * ONE_DAY_SECONDS;

// The maximum dissolve delay allowed for a neuron.
pub const MAX_DISSOLVE_DELAY_SECONDS: u64 = 8 * ONE_YEAR_SECONDS;

Expand Down Expand Up @@ -186,6 +189,13 @@ pub const REWARD_DISTRIBUTION_PERIOD_SECONDS: u64 = ONE_DAY_SECONDS;
/// The maximum number of neurons supported.
pub const MAX_NUMBER_OF_NEURONS: usize = 350_000;

// Spawning is exempted from rate limiting, so we don't need large of a limit here.
pub const MAX_SUSTAINED_NEURONS_PER_HOUR: u64 = 15;

pub const MINIMUM_SECONDS_BETWEEN_ALLOWANCE_INCREASE: u64 = 3600 / MAX_SUSTAINED_NEURONS_PER_HOUR;

pub const MAX_NEURON_CREATION_SPIKE: u64 = MAX_SUSTAINED_NEURONS_PER_HOUR * 8;

/// The maximum number results returned by the method `list_proposals`.
pub const MAX_LIST_PROPOSAL_RESULTS: u32 = 100;

Expand Down Expand Up @@ -1576,6 +1586,23 @@ impl LedgerUpdateLock {
}
}

struct NeuronRateLimits {
/// The number of neurons that can be created.
available_allowances: u64,
/// The last time the number of neurons that can be created was increased.
last_allowance_increase: u64,
}

impl Default for NeuronRateLimits {
fn default() -> Self {
Self {
// Post-upgrade, we reset to max neurons per hour
available_allowances: MAX_NEURON_CREATION_SPIKE,
last_allowance_increase: 0,
}
}
}

/// The `Governance` canister implements the full public interface of the
/// IC's governance system.
pub struct Governance {
Expand Down Expand Up @@ -1612,6 +1639,9 @@ pub struct Governance {

/// Scope guard for minting node provider rewards.
minting_node_provider_rewards: bool,

/// Current neurons available to create
neuron_rate_limits: NeuronRateLimits,
}

pub fn governance_minting_account() -> AccountIdentifier {
Expand Down Expand Up @@ -1827,6 +1857,7 @@ impl Governance {
latest_gc_num_proposals: 0,
neuron_data_validator: NeuronDataValidator::new(),
minting_node_provider_rewards: false,
neuron_rate_limits: NeuronRateLimits::default(),
}
}

Expand Down Expand Up @@ -1857,6 +1888,7 @@ impl Governance {
latest_gc_num_proposals: 0,
neuron_data_validator: NeuronDataValidator::new(),
minting_node_provider_rewards: false,
neuron_rate_limits: NeuronRateLimits::default(),
}
}

Expand Down Expand Up @@ -2097,7 +2129,12 @@ impl Governance {
/// - the maximum number of neurons has been reached, or
/// - the given `neuron_id` already exists in `self.neuron_store.neurons`, or
/// - the neuron's controller `PrincipalId` is not self-authenticating.
fn add_neuron(&mut self, neuron_id: u64, neuron: Neuron) -> Result<(), GovernanceError> {
fn add_neuron(
&mut self,
neuron_id: u64,
neuron: Neuron,
with_rate_limits: bool,
) -> Result<(), GovernanceError> {
if neuron_id == 0 {
return Err(GovernanceError::new_with_message(
ErrorType::PreconditionFailed,
Expand Down Expand Up @@ -2136,8 +2173,23 @@ impl Governance {
));
}

if with_rate_limits && self.neuron_rate_limits.available_allowances < 1 {
return Err(GovernanceError::new_with_message(
ErrorType::Unavailable,
"Reached maximum number of neurons that can be created in this hour. \
Please wait and try again later.",
));
}

self.neuron_store.add_neuron(neuron)?;

if with_rate_limits {
self.neuron_rate_limits.available_allowances = self
.neuron_rate_limits
.available_allowances
.saturating_sub(1);
}

Ok(())
}

Expand All @@ -2157,6 +2209,7 @@ impl Governance {
));
}
self.neuron_store.remove_neuron(&neuron_id);
self.neuron_rate_limits.available_allowances += 1;

Ok(())
}
Expand Down Expand Up @@ -2723,7 +2776,7 @@ impl Governance {
// acquiring the lock. Indeed, in case there is already a pending
// command, we return without state rollback. If we had already created
// the embryo, it would not be garbage collected.
self.add_neuron(child_nid.id, child_neuron.clone())?;
self.add_neuron(child_nid.id, child_neuron.clone(), true)?;

// Do the transfer for the parent first, to avoid double spending.
self.neuron_store.with_neuron_mut(id, |parent_neuron| {
Expand Down Expand Up @@ -3124,7 +3177,7 @@ impl Governance {
.build();

// `add_neuron` will verify that `child_neuron.controller` `is_self_authenticating()`, so we don't need to check it here.
self.add_neuron(child_nid.id, child_neuron)?;
self.add_neuron(child_nid.id, child_neuron, false)?;

// Get the parent neuron again, but this time mutable references.
self.with_neuron_mut(id, |parent_neuron| {
Expand Down Expand Up @@ -3412,7 +3465,7 @@ impl Governance {
.with_kyc_verified(parent_neuron.kyc_verified)
.build();

self.add_neuron(child_nid.id, child_neuron.clone())?;
self.add_neuron(child_nid.id, child_neuron.clone(), true)?;

// Add the child neuron to the set of neurons undergoing ledger updates.
let _child_lock = self.lock_neuron_for_command(child_nid.id, in_flight_command.clone())?;
Expand Down Expand Up @@ -4172,7 +4225,7 @@ impl Governance {
.with_kyc_verified(true)
.build();

self.add_neuron(nid.id, neuron)
self.add_neuron(nid.id, neuron, false)
}
Some(RewardMode::RewardToAccount(reward_to_account)) => {
// We are not creating a neuron, just transferring funds.
Expand Down Expand Up @@ -6170,8 +6223,9 @@ impl Governance {
nid,
subaccount,
controller,
DissolveStateAndAge::DissolvingOrDissolved {
when_dissolved_timestamp_seconds: now,
DissolveStateAndAge::NotDissolving {
dissolve_delay_seconds: INITIAL_NEURON_DISSOLVE_DELAY,
aging_since_timestamp_seconds: now,
},
now,
)
Expand All @@ -6180,7 +6234,7 @@ impl Governance {
.build();

// This also verifies that there are not too many neurons already.
self.add_neuron(nid.id, neuron.clone())?;
self.add_neuron(nid.id, neuron.clone(), true)?;

let _neuron_lock = self.lock_neuron_for_command(
nid.id,
Expand Down Expand Up @@ -6461,6 +6515,30 @@ impl Governance {
.maybe_validate(self.env.now(), &self.neuron_store);
}

/// Increment neuron allowances if enough time has passed.
fn maybe_increase_neuron_allowances(&mut self) {
// We increase the allowance over the maximum per hour to account
// for natural variations in rates, but not leaving too much spare capacity.
if self.neuron_rate_limits.available_allowances < MAX_NEURON_CREATION_SPIKE {
let time_elapsed_since_last_allowance_increase = self
.env
.now()
.saturating_sub(self.neuron_rate_limits.last_allowance_increase);

// Heartbeats should run frequently enough that the allowances increase fairly close
// to what's intended. However, some variation is acceptable.
if time_elapsed_since_last_allowance_increase
>= MINIMUM_SECONDS_BETWEEN_ALLOWANCE_INCREASE
{
self.neuron_rate_limits.available_allowances = self
.neuron_rate_limits
.available_allowances
.saturating_add(1);
self.neuron_rate_limits.last_allowance_increase = self.env.now();
}
}
}

/// Triggers a reward distribution event if enough time has passed since
/// the last one. This is intended to be called by a cron
/// process.
Expand Down Expand Up @@ -6540,6 +6618,7 @@ impl Governance {
self.maybe_gc();
self.maybe_run_migrations();
self.maybe_run_validations();
self.maybe_increase_neuron_allowances();
}

fn should_update_maturity_modulation(&self) -> bool {
Expand Down
Loading

0 comments on commit 4bed17b

Please sign in to comment.