Skip to content
This repository has been archived by the owner on Jan 22, 2025. It is now read-only.

Commit

Permalink
Clean up signers (#2477)
Browse files Browse the repository at this point in the history
- remove unused functions
- remove `new` prefix from stuff I duplicated and have now removed the original of
- remove imports of old functions from `@solana/transaction`

This should leave the final new API for `signers`.
  • Loading branch information
mcintyre94 authored Apr 12, 2024
1 parent 907cf1f commit 727a511
Show file tree
Hide file tree
Showing 20 changed files with 105 additions and 372 deletions.
50 changes: 16 additions & 34 deletions packages/signers/src/__tests__/__setup__.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,13 @@ import { AccountRole, IInstruction } from '@solana/instructions';
import type { Blockhash } from '@solana/rpc-types';
import { CompilableTransactionMessage } from '@solana/transaction-messages';
import {
appendTransactionInstruction,
CompilableTransaction,
createTransaction,
setTransactionFeePayer,
setTransactionLifetimeUsingBlockhash,
} from '@solana/transactions';
appendTransactionMessageInstruction,
createTransactionMessage,
setTransactionMessageFeePayer,
setTransactionMessageLifetimeUsingBlockhash,
} from '@solana/transaction-messages';

import {
IAccountSignerMeta,
IInstructionWithSigners,
ITransactionMessageWithSigners,
ITransactionWithSigners,
} from '../account-signer-meta';
import { IAccountSignerMeta, IInstructionWithSigners, ITransactionMessageWithSigners } from '../account-signer-meta';
import { MessageModifyingSigner } from '../message-modifying-signer';
import { MessagePartialSigner } from '../message-partial-signer';
import { TransactionModifyingSigner } from '../transaction-modifying-signer';
Expand All @@ -33,28 +27,16 @@ export function createMockInstructionWithSigners(signers: TransactionSigner[]):
};
}

export function createMockTransactionWithSigners(
signers: TransactionSigner[],
): CompilableTransaction & ITransactionWithSigners {
const transaction = createTransaction({ version: 0 });
const transactionWithFeePayer = setTransactionFeePayer(signers[0]?.address ?? '1111', transaction);
const compilableTransaction = setTransactionLifetimeUsingBlockhash(
{ blockhash: 'dummy_blockhash' as Blockhash, lastValidBlockHeight: 42n },
transactionWithFeePayer,
);
return appendTransactionInstruction(createMockInstructionWithSigners(signers), compilableTransaction);
}

export function createMockTransactionMessageWithSigners(
signers: TransactionSigner[],
): CompilableTransactionMessage & ITransactionMessageWithSigners {
const transaction = createTransaction({ version: 0 });
const transactionWithFeePayer = setTransactionFeePayer(signers[0]?.address ?? '1111', transaction);
const compilableTransaction = setTransactionLifetimeUsingBlockhash(
const transaction = createTransactionMessage({ version: 0 });
const transactionWithFeePayer = setTransactionMessageFeePayer(signers[0]?.address ?? '1111', transaction);
const compilableTransaction = setTransactionMessageLifetimeUsingBlockhash(
{ blockhash: 'dummy_blockhash' as Blockhash, lastValidBlockHeight: 42n },
transactionWithFeePayer,
);
return appendTransactionInstruction(createMockInstructionWithSigners(signers), compilableTransaction);
return appendTransactionMessageInstruction(createMockInstructionWithSigners(signers), compilableTransaction);
}

export function createMockMessagePartialSigner(address: Address): MessagePartialSigner & { signMessages: jest.Mock } {
Expand All @@ -69,20 +51,20 @@ export function createMockMessageModifyingSigner(

export function createMockTransactionPartialSigner(
address: Address,
): TransactionPartialSigner & { newSignTransactions: jest.Mock; signTransactions: jest.Mock } {
return { address, newSignTransactions: jest.fn(), signTransactions: jest.fn() };
): TransactionPartialSigner & { signTransactions: jest.Mock } {
return { address, signTransactions: jest.fn() };
}

export function createMockTransactionModifyingSigner(
address: Address,
): TransactionModifyingSigner & { modifyAndSignTransactions: jest.Mock; newModifyAndSignTransactions: jest.Mock } {
return { address, modifyAndSignTransactions: jest.fn(), newModifyAndSignTransactions: jest.fn() };
): TransactionModifyingSigner & { modifyAndSignTransactions: jest.Mock } {
return { address, modifyAndSignTransactions: jest.fn() };
}

export function createMockTransactionSendingSigner(
address: Address,
): TransactionSendingSigner & { newSignAndSendTransactions: jest.Mock; signAndSendTransactions: jest.Mock } {
return { address, newSignAndSendTransactions: jest.fn(), signAndSendTransactions: jest.fn() };
): TransactionSendingSigner & { signAndSendTransactions: jest.Mock } {
return { address, signAndSendTransactions: jest.fn() };
}

export function createMockTransactionCompositeSigner(address: Address) {
Expand Down
52 changes: 1 addition & 51 deletions packages/signers/src/__tests__/account-signer-meta-test.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,13 @@
import { Address } from '@solana/addresses';
import { createTransactionMessage } from '@solana/transaction-messages';
import { createTransaction } from '@solana/transactions';

import {
getSignersFromInstruction,
getSignersFromTransaction,
getSignersFromTransactionMessage,
} from '../account-signer-meta';
import { getSignersFromInstruction, getSignersFromTransactionMessage } from '../account-signer-meta';
import { setTransactionFeePayerSigner } from '../fee-payer-signer';
import {
createMockInstructionWithSigners,
createMockTransactionMessageWithSigners,
createMockTransactionModifyingSigner,
createMockTransactionPartialSigner,
createMockTransactionWithSigners,
} from './__setup__';

describe('getSignersFromInstruction', () => {
Expand Down Expand Up @@ -47,50 +41,6 @@ describe('getSignersFromInstruction', () => {
});
});

describe('getSignersFromTransaction', () => {
it('extracts signers from the account meta of the provided transaction', () => {
// Given a transaction with two signers A and B.
const signerA = createMockTransactionPartialSigner('1111' as Address);
const signerB = createMockTransactionModifyingSigner('2222' as Address);
const transaction = createMockTransactionWithSigners([signerA, signerB]);

// When we extract the signers from the transaction's account metas.
const extractedSigners = getSignersFromTransaction(transaction);

// Then we expect signer A and B to be part of the extracted signers.
expect(extractedSigners).toHaveLength(2);
expect(extractedSigners[0]).toBe(signerA);
expect(extractedSigners[1]).toBe(signerB);
});

it('extracts the fee payer signer of the provided transaction', () => {
// Given a transaction with a signer fee payer.
const feePayerSigner = createMockTransactionPartialSigner('1111' as Address);
const transaction = setTransactionFeePayerSigner(feePayerSigner, createTransaction({ version: 0 }));

// When we extract the signers from the transaction.
const extractedSigners = getSignersFromTransaction(transaction);

// Then we expect the extracted signers to contain the fee payer signer.
expect(extractedSigners).toHaveLength(1);
expect(extractedSigners[0]).toBe(feePayerSigner);
});

it('removes duplicated signers', () => {
// Given a transaction with duplicated signers using the same reference.
const signerA = createMockTransactionPartialSigner('1111' as Address);
const signerB = createMockTransactionModifyingSigner('2222' as Address);
const transaction = createMockTransactionWithSigners([signerA, signerB, signerA, signerA]);

// When we extract the signers from the transaction's account metas.
const extractedSigners = getSignersFromTransaction(transaction);

// Then we expect the extracted signers to be deduplicated.
expect(extractedSigners).toHaveLength(2);
expect(extractedSigners.map(signer => signer.address).sort()).toStrictEqual(['1111', '2222']);
});
});

describe('getSignersFromTransactionMessage', () => {
it('extracts signers from the account meta of the provided transaction', () => {
// Given a transaction with two signers A and B.
Expand Down
8 changes: 4 additions & 4 deletions packages/signers/src/__tests__/add-signers-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import '@solana/test-matchers/toBeFrozenObject';
import { Address } from '@solana/addresses';
import { SOLANA_ERROR__SIGNER__ADDRESS_CANNOT_HAVE_MULTIPLE_SIGNERS, SolanaError } from '@solana/errors';
import { AccountRole, IInstruction } from '@solana/instructions';
import { BaseTransaction } from '@solana/transactions';
import { BaseTransactionMessage } from '@solana/transaction-messages';

import { IAccountSignerMeta, IInstructionWithSigners } from '../account-signer-meta';
import { addSignersToInstruction, addSignersToTransactionMessage } from '../add-signers';
Expand Down Expand Up @@ -169,7 +169,7 @@ describe('addSignersToTransactionMessage', () => {
data: new Uint8Array([]),
programAddress: '9999' as Address,
};
const transaction: BaseTransaction = {
const transaction: BaseTransactionMessage = {
instructions: [instructionA, instructionB],
version: 0,
};
Expand All @@ -192,7 +192,7 @@ describe('addSignersToTransactionMessage', () => {

it('freezes the returned transaction', () => {
// Given a one-instruction transaction with signer account metas.
const transaction: BaseTransaction = {
const transaction: BaseTransactionMessage = {
instructions: [
{
accounts: [{ address: '1111' as Address, role: AccountRole.READONLY_SIGNER }],
Expand All @@ -215,7 +215,7 @@ describe('addSignersToTransactionMessage', () => {

it('returns the transaction as-is if it has no instructions', () => {
// Given transaction with no instructions.
const transaction: BaseTransaction = { instructions: [], version: 0 };
const transaction: BaseTransactionMessage = { instructions: [], version: 0 };

// When we try to add signers to the transaction.
const signer = createMockTransactionPartialSigner('1111' as Address);
Expand Down
53 changes: 3 additions & 50 deletions packages/signers/src/__tests__/keypair-signer-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,7 @@ import '@solana/test-matchers/toBeFrozenObject';
import { address, getAddressFromPublicKey } from '@solana/addresses';
import { SOLANA_ERROR__SIGNER__EXPECTED_KEY_PAIR_SIGNER, SolanaError } from '@solana/errors';
import { generateKeyPair, SignatureBytes, signBytes } from '@solana/keys';
import {
CompilableTransaction,
newPartiallySignTransaction,
NewTransaction,
partiallySignTransaction,
} from '@solana/transactions';
import { newPartiallySignTransaction, NewTransaction } from '@solana/transactions';

import {
assertIsKeyPairSigner,
Expand Down Expand Up @@ -43,7 +38,6 @@ describe('isKeyPairSigner', () => {
const mySigner = {
address: myAddress,
keyPair: getMockCryptoKeyPair(),
newSignTransactions: () => Promise.resolve([]),
signMessages: () => Promise.resolve([]),
signTransactions: () => Promise.resolve([]),
} satisfies KeyPairSigner<'Gp7YgHcJciP4px5FdFnywUiMG4UcfMZV9UagSAZzDxdy'>;
Expand All @@ -62,7 +56,6 @@ describe('assertIsKeyPairSigner', () => {
const mySigner = {
address: myAddress,
keyPair: getMockCryptoKeyPair(),
newSignTransactions: () => Promise.resolve([]),
signMessages: () => Promise.resolve([]),
signTransactions: () => Promise.resolve([]),
} satisfies KeyPairSigner<'Gp7YgHcJciP4px5FdFnywUiMG4UcfMZV9UagSAZzDxdy'>;
Expand Down Expand Up @@ -142,7 +135,7 @@ describe('createSignerFromKeyPair', () => {
expect(jest.mocked(signBytes)).toHaveBeenNthCalledWith(2, myKeyPair.privateKey, messages[1].content);
});

it('signs transactions using the newSignTransactions function', async () => {
it('signs transactions using the signTransactions function', async () => {
expect.assertions(7);

// Given a KeyPairSigner created from a mock CryptoKeyPair.
Expand All @@ -166,7 +159,7 @@ describe('createSignerFromKeyPair', () => {
});

// When we sign both transactions using that signer.
const signatureDictionaries = await mySigner.newSignTransactions(mockTransactions);
const signatureDictionaries = await mySigner.signTransactions(mockTransactions);

// Then the signature directories contain the expected signatures.
expect(signatureDictionaries[0]).toStrictEqual({ [myAddress]: mockSignatures[0] });
Expand All @@ -181,46 +174,6 @@ describe('createSignerFromKeyPair', () => {
expect(jest.mocked(newPartiallySignTransaction)).toHaveBeenNthCalledWith(1, [myKeyPair], mockTransactions[0]);
expect(jest.mocked(newPartiallySignTransaction)).toHaveBeenNthCalledWith(2, [myKeyPair], mockTransactions[1]);
});

it('signs transactions using the signTransactions function', async () => {
expect.assertions(7);

// Given a KeyPairSigner created from a mock CryptoKeyPair.
const myKeyPair = getMockCryptoKeyPair();
const myAddress = address('Gp7YgHcJciP4px5FdFnywUiMG4UcfMZV9UagSAZzDxdy');
jest.mocked(getAddressFromPublicKey).mockResolvedValueOnce(myAddress);
const mySigner = await createSignerFromKeyPair(myKeyPair);

// And given we have a couple of mock transactions to sign.
const mockTransactions = [{} as CompilableTransaction, {} as CompilableTransaction];

// And given we mock the next two calls of the partiallySignTransaction function.
const mockSignatures = [new Uint8Array([101, 101, 101]), new Uint8Array([201, 201, 201])] as SignatureBytes[];
jest.mocked(partiallySignTransaction).mockResolvedValueOnce({
...mockTransactions[0],
signatures: { [myAddress]: mockSignatures[0] },
});
jest.mocked(partiallySignTransaction).mockResolvedValueOnce({
...mockTransactions[1],
signatures: { [myAddress]: mockSignatures[1] },
});

// When we sign both transactions using that signer.
const signatureDictionaries = await mySigner.signTransactions(mockTransactions);

// Then the signature directories contain the expected signatures.
expect(signatureDictionaries[0]).toStrictEqual({ [myAddress]: mockSignatures[0] });
expect(signatureDictionaries[1]).toStrictEqual({ [myAddress]: mockSignatures[1] });

// And the signature directories are frozen.
expect(signatureDictionaries[0]).toBeFrozenObject();
expect(signatureDictionaries[1]).toBeFrozenObject();

// And partiallySignTransaction was called twice with the expected parameters.
expect(jest.mocked(partiallySignTransaction)).toHaveBeenCalledTimes(2);
expect(jest.mocked(partiallySignTransaction)).toHaveBeenNthCalledWith(1, [myKeyPair], mockTransactions[0]);
expect(jest.mocked(partiallySignTransaction)).toHaveBeenNthCalledWith(2, [myKeyPair], mockTransactions[1]);
});
});

describe('generateKeyPairSigner', () => {
Expand Down
6 changes: 3 additions & 3 deletions packages/signers/src/__tests__/noop-signer-test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import '@solana/test-matchers/toBeFrozenObject';

import { address } from '@solana/addresses';
import { CompilableTransaction, NewTransaction } from '@solana/transactions';
import { NewTransaction } from '@solana/transactions';

import { createNoopSigner, NoopSigner } from '../noop-signer';
import { createSignableMessage } from '../signable-message';
Expand Down Expand Up @@ -55,7 +55,7 @@ describe('createNoopSigner', () => {
const mockTransactions = [{} as NewTransaction, {} as NewTransaction];

// When we sign both transactions using that signer.
const signatureDictionaries = await mySigner.newSignTransactions(mockTransactions);
const signatureDictionaries = await mySigner.signTransactions(mockTransactions);

// Then the signature directories are empty and frozen.
expect(signatureDictionaries[0]).toStrictEqual({});
Expand All @@ -71,7 +71,7 @@ describe('createNoopSigner', () => {
const mySigner = createNoopSigner(address('Gp7YgHcJciP4px5FdFnywUiMG4UcfMZV9UagSAZzDxdy'));

// And given we have a couple of mock transactions to sign.
const mockTransactions = [{} as CompilableTransaction, {} as CompilableTransaction];
const mockTransactions = [{} as NewTransaction, {} as NewTransaction];

// When we sign both transactions using that signer.
const signatureDictionaries = await mySigner.signTransactions(mockTransactions);
Expand Down
Loading

0 comments on commit 727a511

Please sign in to comment.