feat: Add compatibility for publicKeys #5895
Conversation
Ran into an issue where I was passing web3.publicKeys to createMintToInstruction but received an anonymous.toString() is undefined error.
The error was that the docs allowed a (signer | publicKey) [ ] but the function only works with a signer[ ], my fix was to move my publicKeys into an object. {publicKey: myKey}
I believe the changes to the logic in this file will resolve the issue for anyone encountering it in the future
There was a problem hiding this comment.
Thanks for reporting this and submitting a fix!
I wonder if the issue had something to do with a dual-package error. TL/DR, if two different versions of @solana/web3.js are somehow being added to the dependency tree, then instanceof is going to fail. Since the conditional defaults to a signer anytime instanceof PublicKey fails, that seems to make sense to me.
It looks like your fix will work, but I suggested a way to write this without using instanceof at all.
| isSigner: true, | ||
| isWritable: false, | ||
| }); | ||
| if (signer instanceof PublicKey || signer instanceof Keypair) { |
There was a problem hiding this comment.
All Keypair are Signer but not all Signer are Keypair.
There was a problem hiding this comment.
@Gitmazter if we're going to proceed with changes, instanceof should also be revised here.
Co-authored-by: Joe C <[email protected]>
|
Will add comment this here as well outside of the oudated conversation: This is a better/simpler solution, will commit it! Thank you for paying attention to my PR :) Still new to cooperating over Github so it's definetly easing my social anxiety lol. After reflecting on your comments the two versions of web3.js argument sounds the most likely to me, i think it applies to wallet adapters as well since I'm using the phantom adapter as part of my project which isnt named PublicKey but is named after a 2 byte hash e.g. ve{}|ey{} etc. Although I did try to convert these before submitting my pr but the problem remained. |
You are welcome to contribute here, mate.
Is this issue related to the fix you've PR'ed here? |
| isSigner: true, | ||
| isWritable: false, | ||
| }); | ||
| if (signer instanceof PublicKey || signer instanceof Keypair) { |
There was a problem hiding this comment.
@Gitmazter if we're going to proceed with changes, instanceof should also be revised here.
| else if (signer.toString()) { | ||
| try { | ||
| const compatiblePubkey = new PublicKey(signer.toString()) | ||
| keys.push({ | ||
| pubkey: compatiblePubkey, | ||
| isSigner: true, | ||
| isWritable: false, | ||
| }); | ||
| } | ||
| catch (e) { | ||
| // not a pubkey | ||
| } | ||
| } |
There was a problem hiding this comment.
Why a try-catch here with no catch?
github-actions
bot
added
the
stale
Ran into an issue where I was passing web3.publicKeys to createMintToInstruction with 3 multiSigners but received an anonymous.toString() is undefined error while trying to use serializeMessage() for offline signing.
The error was that the docs allowed a (signer | publicKey) [ ] but the function only works with a signer[ ], my fix was to move my web3.publicKeys into an object. {publicKey: myKey} which allowed me to serialize the instruction.
What I assume happened was that the logic
pubkey: signer instanceof PublicKey ? signer : signer.publicKeydid not work as intended and saw my publicKeys as signers and thus tried publicKey.publicKey.toString()
I believe the changes to the logic in this file will resolve the issue for anyone encountering it in the future.
I'm not entirely familiar with the SPL library yet so I'd be glad for suggestions for how to improve my PR or just have a discussion if anything else might have caused it