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

feat: add support for creating version 0 transactions #27142

Merged
jstarry merged 6 commits into from
Aug 25, 2022
Merged

feat: add support for creating version 0 transactions #27142

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a1250b0
feat: add support for version 0 transactions
jstarry Aug 15, 2022
f3419b1
chore: feedback
jstarry Aug 18, 2022
6818741
chore: update VersionedMessage type
jstarry Aug 18, 2022
38eb503
chore: use literals for version getter
jstarry Aug 18, 2022
c657363
chore: fix lint error
jstarry Aug 22, 2022
e92391c
chore: switch to VersionedMessage.deserialize
jstarry Aug 22, 2022
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
7 changes: 7 additions & 0 deletions web3.js/src/layout.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,13 @@ export const publicKey = (property: string = 'publicKey') => {
return BufferLayout.blob(32, property);
};

/**
* Layout for a signature
*/
export const signature = (property: string = 'signature') => {
return BufferLayout.blob(64, property);
};

Comment on lines +11 to +17
Copy link
Copy Markdown
Contributor

@jordaaash jordaaash Aug 17, 2022

Choose a reason for hiding this comment

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

Should we add this to https://github.com/solana-labs/buffer-layout-utils/blob/master/src/web3.ts and consume it here? Could move most or all of these into it and eliminate some duplication

Copy link
Copy Markdown
Contributor Author

@jstarry jstarry Aug 18, 2022

Choose a reason for hiding this comment

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

I'd be happy with that but I don't feel it's a big priority for simple layouts for signatures and public keys

/**
* Layout for a 64bit unsigned value
*/
Expand Down
25 changes: 19 additions & 6 deletions web3.js/src/message/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
import {PublicKey} from '../publickey';

export * from './legacy';
export * from './versioned';
export * from './v0';

/**
* The message header, identifying signed and read-only account
Expand All @@ -15,18 +19,27 @@ export type MessageHeader = {
numReadonlyUnsignedAccounts: number;
};

/**
* An address table lookup used to load additional accounts
*/
export type MessageAddressTableLookup = {
accountKey: PublicKey;
writableIndexes: Array<number>;
readonlyIndexes: Array<number>;
Comment on lines +27 to +28
Copy link
Copy Markdown
Contributor

@steveluscher steveluscher Aug 25, 2022

Choose a reason for hiding this comment

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

I was looking for a way that you could architect this type such that it was impossible to screw it up. As written, you could create malformed data like this:

const matl: MessageAddressTableLookup = {
  accountKey: /* ... */,
  writableIndexes: [0, 1],
  readonlyIndexes: [1, 2],
};

If the storage was, instead, implemented as a sparse array, it would be impossible to overlap a readable/writeable index.

type Readable = 0;  // or 'r'
type Writeable = 1;  // or 'w'
type IndexesWithWriteability = (Readable | Writeable | undefined)[];
const indexes: IndexesWithWriteability = [];
indexes[0] = 1;
indexes[1] = 1;
indexes[1] = 0;  // OVERWRITTEN! Note how overlapping index 1 is impossible here.
indexes[2] = 0;

Then you'd do stuff like this at the point of serialization:

const readonlyIndexes = [];
const writeableIndexes = [];
indexes.forEach((writeability, index) => {
  if (writeability === 0) {
    readonlyIndexes.push(index);
  } else if (writeability === 1) {
    writeableIndexes.push(index);
  }
});
const writeableIndexesLength = shortvec.encodeLength(
  encodedWriteableIndexesLength,
  writeableIndexes.length,
);

Copy link
Copy Markdown
Contributor Author

@jstarry jstarry Aug 26, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Oh yeah great point, it's definitely possible to create invalid transaction messages with the MessageV0 constructor. This MessageAddressTableLookup type directly maps to the serialization format of v0 messages (MessageV0 is just a barebones data class) so it's not meant to be created or handled directly by devs. In the draft PR here you can see that the construction of v0 messages has a high level API MessageV0.compile(..) which will build these lookup data structures internally in a valid way:

solana/web3.js/src/message/v0.ts

Lines 71 to 80 in 25d2566

const lookupTableAccounts = args.addressLookupTableAccounts || [];
for (const lookupTable of lookupTableAccounts) {
const extractResult = compiledKeys.extractTableLookup(lookupTable);
if (extractResult !== undefined) {
const [addressTableLookup, loadedAddresses] = extractResult;
addressTableLookups.push(addressTableLookup);
allLoadedAddresses.writable.push(...loadedAddresses.writable);
allLoadedAddresses.readonly.push(...loadedAddresses.readonly);
}
}

solana/web3.js/src/message/compiled-keys.ts

Lines 104 to 123 in 25d2566

extractTableLookup(
lookupTable: AddressLookupTableAccount,
): [MessageAddressTableLookup, LoadedAddresses] | undefined {
const [writableIndexes, drainedWritableKeys] =
this.drainKeysFoundInLookupTable(
lookupTable.state.addresses,
keyMeta =>
!keyMeta.isSigner && !keyMeta.isInvoked && keyMeta.isWritable,
);
const [readonlyIndexes, drainedReadonlyKeys] =
this.drainKeysFoundInLookupTable(
lookupTable.state.addresses,
keyMeta =>
!keyMeta.isSigner && !keyMeta.isInvoked && !keyMeta.isWritable,
);
// Don't extract lookup if no keys were found
if (writableIndexes.length === 0 && readonlyIndexes.length === 0) {
return;
}

Copy link
Copy Markdown
Contributor

@steveluscher steveluscher Aug 30, 2022

Choose a reason for hiding this comment

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

Yeah, only thing is that MessageV0 is exported in #27213, so by Murphy's Law someone will definitely eventually create one with hand rolled indices that overlap.

Copy link
Copy Markdown
Contributor Author

@jstarry jstarry Aug 31, 2022

Choose a reason for hiding this comment

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

I think that's fine. The same holds for any other field in MessageV0... staticAccountKeys could have dup keys, instructions could reference invalid account indexes, the header values could be contradictory, etc.

};

/**
* An instruction to execute by a program
*
* @property {number} programIdIndex
* @property {number[]} accounts
* @property {string} data
* @property {number[]} accountKeyIndexes
* @property {Uint8Array} data
*/
export type CompiledInstruction = {
export type MessageCompiledInstruction = {
/** Index into the transaction keys array indicating the program account that executes this instruction */
programIdIndex: number;
/** Ordered indices into the transaction keys array indicating which accounts to pass to the program */
accounts: number[];
/** The program input data encoded as base 58 */
data: string;
accountKeyIndexes: number[];
Copy link
Copy Markdown
Contributor

@jordaaash jordaaash Aug 17, 2022

Choose a reason for hiding this comment

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

Feels like this should be accounts if it represents the same thing as in CompiledInstruction? MessageCompiledInstruction is a bit confusing as a name (it's just a CompiledInstruction with the data raw instead of encoded)

Copy link
Copy Markdown
Contributor Author

@jstarry jstarry Aug 18, 2022

Choose a reason for hiding this comment

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

MessageCompiledInstruction is meant to be the successor to CompiledInstruction. Encoding the data is unnecessary and inefficient so that was the main reason I wanted to move away from CompiledInstruction. The accounts -> accountKeyIndexes isn't as big of a deal but I feel that it's more clear and descriptive.

/** The program input data */
data: Uint8Array;
};
55 changes: 53 additions & 2 deletions web3.js/src/message/legacy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,30 @@ import * as BufferLayout from '@solana/buffer-layout';
import {PublicKey, PUBLIC_KEY_LENGTH} from '../publickey';
import type {Blockhash} from '../blockhash';
import * as Layout from '../layout';
import {PACKET_DATA_SIZE} from '../transaction/constants';
import {PACKET_DATA_SIZE, VERSION_PREFIX_MASK} from '../transaction/constants';
import * as shortvec from '../utils/shortvec-encoding';
import {toBuffer} from '../utils/to-buffer';
import {CompiledInstruction, MessageHeader} from './index';
import {
MessageHeader,
MessageAddressTableLookup,
MessageCompiledInstruction,
} from './index';

/**
* An instruction to execute by a program
*
* @property {number} programIdIndex
* @property {number[]} accounts
* @property {string} data
*/
export type CompiledInstruction = {
/** Index into the transaction keys array indicating the program account that executes this instruction */
programIdIndex: number;
/** Ordered indices into the transaction keys array indicating which accounts to pass to the program */
accounts: number[];
/** The program input data encoded as base 58 */
data: string;
};

/**
* Message constructor arguments
Expand Down Expand Up @@ -51,6 +71,28 @@ export class Message {
);
}

get version(): 'legacy' {
return 'legacy';
}

get staticAccountKeys(): Array<PublicKey> {
return this.accountKeys;
}

get compiledInstructions(): Array<MessageCompiledInstruction> {
return this.instructions.map(
(ix): MessageCompiledInstruction => ({
programIdIndex: ix.programIdIndex,
accountKeyIndexes: ix.accounts,
data: bs58.decode(ix.data),
}),
);
}

get addressTableLookups(): Array<MessageAddressTableLookup> {
return [];
}

isAccountSigner(index: number): boolean {
return index < this.header.numRequiredSignatures;
}
Expand Down Expand Up @@ -191,6 +233,15 @@ export class Message {
let byteArray = [...buffer];

const numRequiredSignatures = byteArray.shift() as number;
if (
numRequiredSignatures !==
(numRequiredSignatures & VERSION_PREFIX_MASK)
) {
throw new Error(
'Versioned messages must be deserialized with VersionedMessage.deserialize()',
);
}

const numReadonlySignedAccounts = byteArray.shift() as number;
const numReadonlyUnsignedAccounts = byteArray.shift() as number;

Expand Down
Loading