Skip to content

Commit

Permalink
Chore: Change "?" for "Option" for optional fields (#480)
Browse files Browse the repository at this point in the history
# Motivation

Using `?` is not the same as `Option`. With `?` the field is optional,
but with `Option`, the field is mandatory, but the value can be
`undefined`.

This small distinction is important when we render the information of
proposals in NNS Dapp. We convert the actions to camelcase and remove
`[] | [T]`.

If we use `?`, we might forget to convert it and it will never show up
in NNS Dapp. By using `Option`, we make sure that the field will always
we present in the converters.

You can see the example in the `params`, `maxDirectParticipationIcpE8s`
and `minDirectParticipationIcpE8s` fields for the (deprecated)
`OpenSnsTokenSwap` proposal action that I had to change when I moved
from `?` to `Option`.

# Changes

* Change the `?` in `types/governance_converters.ts` for `Option`. Only
for the fields related to proposals.

# Tests

* Fix a converter after changes.

# Todos

- [x] Add entry to changelog (if necessary).
  • Loading branch information
lmuntaner committed Nov 22, 2023
1 parent b627be1 commit 63d29e5
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 71 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

## Features

- Substitute `?` fields with `Option` fields in the converters related to NNS proposals.

# Release.2023.11.21-1400Z

## Overview
Expand Down
34 changes: 19 additions & 15 deletions packages/nns/src/canisters/governance/response.converters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -460,21 +460,25 @@ const toAction = (action: RawAction): Action => {
targetSwapCanisterId: fromNullable(
OpenSnsTokenSwap.target_swap_canister_id,
),
...(params !== undefined && {
params: {
minParticipantIcpE8s: params.min_participant_icp_e8s,
maxIcpE8s: params.max_icp_e8s,
swapDueTimestampSeconds: params.swap_due_timestamp_seconds,
minParticipants: params.min_participants,
snsTokenE8s: params.sns_token_e8s,
maxParticipantIcpE8s: params.max_participant_icp_e8s,
minIcpE8s: params.min_icp_e8s,
saleDelaySeconds: fromNullable(params.sale_delay_seconds),
neuronBasketConstructionParameters: fromNullable(
params.neuron_basket_construction_parameters,
),
},
}),
params: params && {
minParticipantIcpE8s: params.min_participant_icp_e8s,
maxIcpE8s: params.max_icp_e8s,
swapDueTimestampSeconds: params.swap_due_timestamp_seconds,
minParticipants: params.min_participants,
snsTokenE8s: params.sns_token_e8s,
maxParticipantIcpE8s: params.max_participant_icp_e8s,
minIcpE8s: params.min_icp_e8s,
saleDelaySeconds: fromNullable(params.sale_delay_seconds),
neuronBasketConstructionParameters: fromNullable(
params.neuron_basket_construction_parameters,
),
maxDirectParticipationIcpE8s: fromNullable(
params.max_direct_participation_icp_e8s,
),
minDirectParticipationIcpE8s: fromNullable(
params.min_direct_participation_icp_e8s,
),
},
},
};
}
Expand Down
112 changes: 56 additions & 56 deletions packages/nns/src/types/governance_converters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ export interface MergeRequest {
targetNeuronId: NeuronId;
}
export interface StakeMaturity {
percentageToStake?: number;
percentageToStake: Option<number>;
}
export interface MergeMaturity {
percentageToMerge: number;
Expand All @@ -217,34 +217,34 @@ export interface Motion {
motionText: string;
}
export interface OpenSnsTokenSwap {
communityFundInvestmentE8s?: bigint;
targetSwapCanisterId?: Principal;
params?: {
communityFundInvestmentE8s: Option<bigint>;
targetSwapCanisterId: Option<Principal>;
params: Option<{
minParticipantIcpE8s: bigint;
maxIcpE8s: bigint;
swapDueTimestampSeconds: bigint;
minParticipants: number;
snsTokenE8s: bigint;
maxParticipantIcpE8s: bigint;
minIcpE8s: bigint;
saleDelaySeconds?: bigint;
neuronBasketConstructionParameters?: {
saleDelaySeconds: Option<bigint>;
neuronBasketConstructionParameters: Option<{
// Keep snake case to avoid having to convert back and forth.
dissolve_delay_interval_seconds: bigint;
count: bigint;
};
maxDirectParticipationIcpE8s?: bigint;
minDirectParticipationIcpE8s?: bigint;
};
}>;
maxDirectParticipationIcpE8s: Option<bigint>;
minDirectParticipationIcpE8s: Option<bigint>;
}>;
}
export interface SetSnsTokenSwapOpenTimeWindow {
request?: {
openTimeWindow?: {
request: Option<{
openTimeWindow: Option<{
startTimestampSeconds: bigint;
endTimestampSeconds: bigint;
};
};
swapCanisterId?: string;
}>;
}>;
swapCanisterId: Option<string>;
}
export interface NetworkEconomics {
neuronMinimumStake: E8s;
Expand Down Expand Up @@ -503,58 +503,58 @@ export interface ListNodeProvidersResponse {
}

export interface Percentage {
basisPoints?: bigint;
basisPoints: Option<bigint>;
}

export interface Duration {
seconds?: bigint;
seconds: Option<bigint>;
}

export interface GlobalTimeOfDay {
secondsAfterUtcMidnight?: bigint;
secondsAfterUtcMidnight: Option<bigint>;
}

export interface Countries {
isoCodes: Array<string>;
}

export interface Tokens {
e8s?: bigint;
e8s: Option<bigint>;
}

export interface Image {
base64Encoding?: string;
base64Encoding: Option<string>;
}

export interface LedgerParameters {
transactionFee?: Tokens;
tokenSymbol?: string;
tokenLogo?: Image;
tokenName?: string;
transactionFee: Option<Tokens>;
tokenSymbol: Option<string>;
tokenLogo: Option<Image>;
tokenName: Option<string>;
}

export interface VotingRewardParameters {
rewardRateTransitionDuration?: Duration;
initialRewardRate?: Percentage;
finalRewardRate?: Percentage;
rewardRateTransitionDuration: Option<Duration>;
initialRewardRate: Option<Percentage>;
finalRewardRate: Option<Percentage>;
}

export interface GovernanceParameters {
neuronMaximumDissolveDelayBonus?: Percentage;
neuronMaximumAgeForAgeBonus?: Duration;
neuronMaximumDissolveDelay?: Duration;
neuronMinimumDissolveDelayToVote?: Duration;
neuronMaximumAgeBonus?: Percentage;
neuronMinimumStake?: Tokens;
proposalWaitForQuietDeadlineIncrease?: Duration;
proposalInitialVotingPeriod?: Duration;
proposalRejectionFee?: Tokens;
votingRewardParameters?: VotingRewardParameters;
neuronMaximumDissolveDelayBonus: Option<Percentage>;
neuronMaximumAgeForAgeBonus: Option<Duration>;
neuronMaximumDissolveDelay: Option<Duration>;
neuronMinimumDissolveDelayToVote: Option<Duration>;
neuronMaximumAgeBonus: Option<Percentage>;
neuronMinimumStake: Option<Tokens>;
proposalWaitForQuietDeadlineIncrease: Option<Duration>;
proposalInitialVotingPeriod: Option<Duration>;
proposalRejectionFee: Option<Tokens>;
votingRewardParameters: Option<VotingRewardParameters>;
}

export interface NeuronBasketConstructionParameters {
dissolveDelayInterval?: Duration;
count?: bigint;
dissolveDelayInterval: Option<Duration>;
count: Option<bigint>;
}
export interface SwapParameters {
minimumParticipants: Option<bigint>;
Expand All @@ -574,36 +574,36 @@ export interface SwapParameters {
}

export interface SwapDistribution {
total?: Tokens;
total: Option<Tokens>;
}

export interface NeuronDistribution {
controller?: PrincipalString;
dissolveDelay?: Duration;
memo?: bigint;
vestingPeriod?: Duration;
stake?: Tokens;
controller: Option<PrincipalString>;
dissolveDelay: Option<Duration>;
memo: Option<bigint>;
vestingPeriod: Option<Duration>;
stake: Option<Tokens>;
}

export interface DeveloperDistribution {
developerNeurons: Array<NeuronDistribution>;
}

export interface InitialTokenDistribution {
treasuryDistribution?: SwapDistribution;
developerDistribution?: DeveloperDistribution;
swapDistribution?: SwapDistribution;
treasuryDistribution: Option<SwapDistribution>;
developerDistribution: Option<DeveloperDistribution>;
swapDistribution: Option<SwapDistribution>;
}

export interface CreateServiceNervousSystem {
url?: string;
governanceParameters?: GovernanceParameters;
url: Option<string>;
governanceParameters: Option<GovernanceParameters>;
fallbackControllerPrincipalIds: Array<PrincipalString>;
logo?: Image;
name?: string;
ledgerParameters?: LedgerParameters;
description?: string;
logo: Option<Image>;
name: Option<string>;
ledgerParameters: Option<LedgerParameters>;
description: Option<string>;
dappCanisters: Array<CanisterIdString>;
swapParameters?: SwapParameters;
initialTokenDistribution?: InitialTokenDistribution;
swapParameters: Option<SwapParameters>;
initialTokenDistribution: Option<InitialTokenDistribution>;
}

0 comments on commit 63d29e5

Please sign in to comment.