Skip to content

fix: make deactivate stake consistent by taking in comma separated keys #1986

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Oct 3, 2024
Merged

fix: make deactivate stake consistent by taking in comma separated keys #1986

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f89f64d
go
guibescos Oct 2, 2024
4210ead
update args
guibescos Oct 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions governance/xc_admin/packages/xc_admin_cli/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,7 @@ multisigCommand(
"Deactivate the delegated stake from the account"
)
.requiredOption(
"-s, --stake-accounts <accounts...>",
"-s, --stake-accounts <comma_separated_stake_account>",
"stake accounts to be deactivated"
)
.action(async (options: any) => {
Expand All @@ -395,10 +395,15 @@ multisigCommand(
const authorizedPubkey: PublicKey = await vault.getVaultAuthorityPDA(
cluster
);
const instructions = options.stakeAccounts.reduce(
(instructions: TransactionInstruction[], stakeAccount: string) => {

const stakeAccounts = options.stakeAccounts
? options.stakeAccounts.split(",").map((m: string) => new PublicKey(m))
: [];

const instructions = stakeAccounts.reduce(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there any reason to use reduce instead of map?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's because StakeProgram.deactivate returns a list of instructions instead of a single instruction. In other words the type system doesn't guarantee that each element in the stakeAccounts list will map to a single instruction (although in practice StakeProgram.deactivate returns only one instruction I believe). Rather than return a list of list of instructions, this method will return a list that's the concatenation of all the instruction lists.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah got it, in general in those cases it's a bit more idiomatic to use flatMap than reduce

(instructions: TransactionInstruction[], stakeAccount: PublicKey) => {
const transaction = StakeProgram.deactivate({
stakePubkey: new PublicKey(stakeAccount),
stakePubkey: stakeAccount,
authorizedPubkey,
});

Expand Down
Loading