Skip to content
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.

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

ts: better public key error msgs #1098

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ incremented for features.

* lang: Add `ErrorCode::AccountNotInitialized` error to separate the situation when the account has the wrong owner from when it does not exist (#[1024](https://github.com/project-serum/anchor/pull/1024))
* lang: Called instructions now log their name by default. This can be turned off with the `no-log-ix-name` flag ([#1057](https://github.com/project-serum/anchor/pull/1057))
* ts: Add better error msgs in the ts client if something wrong (i.e. not a pubkey or a string) is passed in as an account in an instruction accounts object ([#1098](https://github.com/project-serum/anchor/pull/1098))

## [0.18.2] - 2021-11-14

Expand Down
4 changes: 3 additions & 1 deletion ts/src/program/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,10 @@ export function translateAddress(address: Address): PublicKey {
if (typeof address === "string") {
const pk = new PublicKey(address);
return pk;
} else {
} else if (address.constructor.prototype.constructor.name === "PublicKey") {
Copy link
Contributor

Choose a reason for hiding this comment

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

This has introduced some bugs in our production build. I believe this is because JS is minifying the code and changing the name of constructors.

A couple suggestions to avoid this:

  • check if the method toBase58 exists on the object
  • add a name or type to the PublicKey constructor object similar to how Phantom adds isPhantom to window.solana

What do you think? Happy to open a PR that fixes this :)

cc @paul-schaaf

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sorry for that, didnt consider this, my mistake!

fix is already on the way #1138

Copy link
Contributor

Choose a reason for hiding this comment

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

Awesome, thanks for the pointer there @paul-schaaf :)

Choose a reason for hiding this comment

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

I don't have the full picture of the codebase and the issue, just wanted to chime in that some time ago I fixed precisely the same bug replacing such a clause with an instanceofaddress instanceof PublicKey.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we can try again but it didnt work for us before #1101

return address;
} else {
throw new Error("Given address is not a PublicKey nor a string.");
}
}

Expand Down
26 changes: 22 additions & 4 deletions ts/src/program/namespace/instruction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,15 +63,20 @@ export default class InstructionNamespaceFactory {

// Utility fn for ordering the accounts for this instruction.
ix["accounts"] = (accs: Accounts<I["accounts"][number]> | undefined) => {
return InstructionNamespaceFactory.accountsArray(accs, idlIx.accounts);
return InstructionNamespaceFactory.accountsArray(
accs,
idlIx.accounts,
idlIx.name
);
};

return ix;
}

public static accountsArray(
ctx: Accounts | undefined,
accounts: readonly IdlAccountItem[]
accounts: readonly IdlAccountItem[],
ixName?: string
): AccountMeta[] {
if (!ctx) {
return [];
Expand All @@ -86,12 +91,25 @@ export default class InstructionNamespaceFactory {
const rpcAccs = ctx[acc.name] as Accounts;
return InstructionNamespaceFactory.accountsArray(
rpcAccs,
(acc as IdlAccounts).accounts
(acc as IdlAccounts).accounts,
ixName
).flat();
} else {
const account: IdlAccount = acc as IdlAccount;
let pubkey;
try {
pubkey = translateAddress(ctx[acc.name] as Address);
} catch (err) {
throw new Error(
`Wrong input type for account "${
acc.name
}" in the instruction accounts object${
ixName !== undefined ? ' for instruction "' + ixName + '"' : ""
}. Expected PublicKey or string.`
);
}
return {
pubkey: translateAddress(ctx[acc.name] as Address),
pubkey,
isWritable: account.isMut,
isSigner: account.isSigner,
};
Expand Down
6 changes: 5 additions & 1 deletion ts/src/program/namespace/state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,11 @@ export class StateClient<IDL extends Idl> {
ixItem["accounts"] = (accounts) => {
const keys = stateInstructionKeys(programId, provider, m, accounts);
return keys.concat(
InstructionNamespaceFactory.accountsArray(accounts, m.accounts)
InstructionNamespaceFactory.accountsArray(
accounts,
m.accounts,
m.name
)
);
};
// Build transaction method.
Expand Down