Skip to content

Commit

Permalink
feat(sns): Postpone enforcing the 100K SNS neuron limit till swap beg…
Browse files Browse the repository at this point in the history 
…ins (#1332)

This PR lifts an overly strict requirement due to which some SNS
configurations were deemed invalid if it was possible to exceed the 100K
SNS neurons limit for swap participants in the worst case.

[Recently](#924), the maximum number
of swap participants has started to be enforced during swap
participation, so checking the condition upfront is no longer needed for
security, and so we lift it to allow for more SNS configurations.
  • Loading branch information
@aterga
aterga authored Sep 5, 2024
1 parent 6d16531 commit cd153a6
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 100 deletions.
7 changes: 5 additions & 2 deletions rs/nervous_system/integration_tests/tests/constraints_dependencies.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,14 @@ use ic_sns_init::{
// against the sum of intermediate limits set for various types of SNS neurons. These intermediate
// limits are not checked within just one canister, so testing their inter-consistency is done here.
//
// Many SNS neurons may be created after a swap succeeds. The number of such neurons is limited to
// `MAX_NEURONS_FOR_DIRECT_PARTICIPANTS`. This limit is enforced only *during* the swap. In effect,
// this limits the maximum number of swap participants to `MAX_NEURONS_FOR_DIRECT_PARTICIPANTS` /
// #number of SNS neurons per participant (a.k.a., the SNS basket count).
//
// If a `CreateServiceNervousSystem` proposal is valid, its parameters must comply, in particular,
// with the following limits (checked at the time of proposal submission):
// - The number of SNS neurons per basket does not exceed `MAX_SNS_NEURONS_PER_BASKET`.
// - The number of SNS neurons created for direct swap participants in the worst case cannot exceed
// `MAX_NEURONS_FOR_DIRECT_PARTICIPANTS`.
// - The number of SNS neurons granted to the dapp developers doe snot exceed
// `MAX_DEVELOPER_DISTRIBUTION_COUNT`.
//
Expand Down
143 changes: 45 additions & 98 deletions rs/sns/init/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,7 @@ use ic_icrc1_index_ng::{IndexArg, InitArg};
use ic_icrc1_ledger::{InitArgsBuilder as LedgerInitArgsBuilder, LedgerArgument};
use ic_ledger_canister_core::archive::ArchiveOptions;
use ic_ledger_core::Tokens;
use ic_nervous_system_common::{
ledger_validation, DEFAULT_TRANSFER_FEE, E8, MAX_NEURONS_FOR_DIRECT_PARTICIPANTS,
};
use ic_nervous_system_common::{ledger_validation, DEFAULT_TRANSFER_FEE, E8};
use ic_nervous_system_proto::pb::v1::{Canister, Countries};
use ic_nns_constants::{
CYCLES_MINTING_CANISTER_ID, EXCHANGE_RATE_CANISTER_ID, GENESIS_TOKEN_CANISTER_ID,
Expand Down Expand Up @@ -1552,8 +1550,6 @@ impl SnsInitPayload {
/// (8) Required ICP participation amount is big enough to ensure that all participants will
/// end up with enough SNS tokens to form the right number of SNS neurons (after paying for
/// the SNS ledger transaction fee to create each such SNS neuron).
/// (9) The maximum possible number of SNS neurons created for direct participants (in case
/// the swap succeeds) does not exceed `MAX_NEURONS_FOR_DIRECT_PARTICIPANTS`.
///
/// * -- In the context of this function, swap participation-related parameters include:
/// - `min_direct_participation_icp_e8s` - Required ICP amount for the swap to succeed.
Expand Down Expand Up @@ -1708,33 +1704,6 @@ impl SnsInitPayload {
));
}

// (9)
// Conceptually, we want to calculate the following value:
// ```
// let max_sns_neurons_for_direct_participants = {
// let max_participants = max_direct_participation_icp_e8s / min_participant_icp_e8s;
// max_participants * neuron_basket_construction_parameters_count;
// };
// ```
// To minimize rounding errors related to integer division, we first do `*` and then `/`.
let max_sns_neurons_for_direct_participants = max_direct_participation_icp_e8s as u128
* neuron_basket_construction_parameters_count as u128
/ min_participant_icp_e8s as u128;
if max_sns_neurons_for_direct_participants > MAX_NEURONS_FOR_DIRECT_PARTICIPANTS as u128 {
return Err(format!(
"Error: The number of SNS neurons created for direct participants of a successful \
swap ((max_direct_participation_icp_e8s={}) \
* (neuron_basket_construction_parameters_count={}) \
/ (min_participant_icp_e8s={}) = {}) must not exceed \
(MAX_NEURONS_FOR_DIRECT_PARTICIPANTS={}).",
max_direct_participation_icp_e8s,
neuron_basket_construction_parameters_count,
min_participant_icp_e8s,
max_sns_neurons_for_direct_participants,
MAX_NEURONS_FOR_DIRECT_PARTICIPANTS
));
}

Ok(())
}

Expand Down Expand Up @@ -2016,7 +1985,7 @@ mod test {
NeuronsFundParticipationConstraintsValidationError, RestrictedCountriesValidationError,
SnsCanisterIds, SnsInitPayload, ICRC1_TOKEN_LOGO_KEY, MAX_CONFIRMATION_TEXT_LENGTH,
MAX_DAPP_CANISTERS_COUNT, MAX_DIRECT_ICP_CONTRIBUTION_TO_SWAP,
MAX_FALLBACK_CONTROLLER_PRINCIPAL_IDS_COUNT, MAX_NEURONS_FOR_DIRECT_PARTICIPANTS,
MAX_FALLBACK_CONTROLLER_PRINCIPAL_IDS_COUNT,
};
use ic_base_types::{CanisterId, PrincipalId};
use ic_icrc1_ledger::LedgerArgument;
Expand Down Expand Up @@ -3432,36 +3401,39 @@ mod test {
}

#[test]
fn test_number_of_sns_neuorns_for_direct_participants_cannot_exceed_limit() {
fn test_validate_participation_constraints() {
// Common part for the happy and failing scenarios.
let fdvp = FractionalDVP {
swap_distribution: Some(SwapDistribution {
initial_swap_amount_e8s: MAX_DIRECT_ICP_CONTRIBUTION_TO_SWAP,
// Not used in this test.
total_e8s: 0,
}),
// Not used in this test.
developer_distribution: None,
treasury_distribution: None,
airdrop_distribution: None,
};
let sns_init_payload = SnsInitPayload {
max_direct_participation_icp_e8s: Some(MAX_DIRECT_ICP_CONTRIBUTION_TO_SWAP),
min_direct_participation_icp_e8s: Some(1),
max_participant_icp_e8s: Some(MAX_DIRECT_ICP_CONTRIBUTION_TO_SWAP),
min_participants: Some(40_000),
initial_token_distribution: Some(FractionalDeveloperVotingPower(fdvp)),
neuron_basket_construction_parameters: Some(NeuronBasketConstructionParameters {
count: 2,
// Not used in this test.
dissolve_delay_interval_seconds: 0,
}),
neuron_minimum_stake_e8s: Some(1),
transaction_fee_e8s: Some(0),
..SnsInitPayload::with_valid_values_for_testing_pre_execution()
};
// Happy scenario: Test that if the constraints are satisfied, the validation passes.
{
let fdvp = FractionalDVP {
swap_distribution: Some(SwapDistribution {
initial_swap_amount_e8s: MAX_DIRECT_ICP_CONTRIBUTION_TO_SWAP,
// Not used in this test.
total_e8s: 0,
}),
// Not used in this test.
developer_distribution: None,
treasury_distribution: None,
airdrop_distribution: None,
};
let sns_init_payload = SnsInitPayload {
max_direct_participation_icp_e8s: Some(MAX_DIRECT_ICP_CONTRIBUTION_TO_SWAP),
min_direct_participation_icp_e8s: Some(1),
// This value being high enough ensures there wouldn't be too many SNS neurons.
min_participant_icp_e8s: Some(20_000 * E8),
max_participant_icp_e8s: Some(MAX_DIRECT_ICP_CONTRIBUTION_TO_SWAP),
min_participants: Some(40_000),
initial_token_distribution: Some(FractionalDeveloperVotingPower(fdvp)),
neuron_basket_construction_parameters: Some(NeuronBasketConstructionParameters {
count: 2,
// Not used in this test.
dissolve_delay_interval_seconds: 0,
}),
neuron_minimum_stake_e8s: Some(1),
transaction_fee_e8s: Some(0),
..SnsInitPayload::with_valid_values_for_testing_pre_execution()
..sns_init_payload.clone()
};

// Run code under test.
Expand All @@ -3471,59 +3443,34 @@ mod test {
}
// Test that if the constraints are violated, the validation, indeed, fails.
{
let fdvp = FractionalDVP {
swap_distribution: Some(SwapDistribution {
initial_swap_amount_e8s: MAX_DIRECT_ICP_CONTRIBUTION_TO_SWAP,
// Not used in this test.
total_e8s: 0,
}),
// Not used in this test.
developer_distribution: None,
treasury_distribution: None,
airdrop_distribution: None,
};
let sns_init_payload = SnsInitPayload {
max_direct_participation_icp_e8s: Some(MAX_DIRECT_ICP_CONTRIBUTION_TO_SWAP),
min_direct_participation_icp_e8s: Some(1),
// This value being so low ensures there would be too many SNS neurons.
min_participant_icp_e8s: Some(2),
max_participant_icp_e8s: Some(MAX_DIRECT_ICP_CONTRIBUTION_TO_SWAP),
min_participants: Some(40_000),
initial_token_distribution: Some(FractionalDeveloperVotingPower(fdvp)),
neuron_basket_construction_parameters: Some(NeuronBasketConstructionParameters {
count: 2,
// Not used in this test.
dissolve_delay_interval_seconds: 0,
}),
neuron_minimum_stake_e8s: Some(1),
transaction_fee_e8s: Some(0),
..SnsInitPayload::with_valid_values_for_testing_pre_execution()
// This value being so low ensures there would be a problem.
min_participant_icp_e8s: Some(1),
..sns_init_payload
};

// Run code under test.
let error = sns_init_payload
.validate_participation_constraints()
.unwrap_err();
{
let expected_error_fragment =
"number of SNS neurons created for direct participants";
let expected_error_fragment_a = "min_participant_icp_e8s=1 is too small.";
assert!(
error.contains(expected_error_fragment),
error.contains(expected_error_fragment_a),
"Unexpected error: `{}`\nExpected `{}`",
error,
expected_error_fragment
);
}
{
let expected_error_fragment = format!(
"must not exceed (MAX_NEURONS_FOR_DIRECT_PARTICIPANTS={})",
MAX_NEURONS_FOR_DIRECT_PARTICIPANTS
expected_error_fragment_a,
);

let expecrted_error_fragment_b = "the following inequality must hold: \
min_participant_icp_e8s >= neuron_basket_count \
* (neuron_minimum_stake_e8s + transaction_fee_e8s) \
* max_direct_participation_icp_e8s / initial_swap_amount_e8s";
assert!(
error.contains(&expected_error_fragment),
"Unexpected error: {}\nExpected `{}`",
error.contains(expecrted_error_fragment_b),
"Unexpected error: `{}`\nExpected `{}`",
error,
expected_error_fragment
expecrted_error_fragment_b,
);
}
}
Expand Down

0 comments on commit cd153a6

Please sign in to comment.