solana-labs / solana-web3.js

Solana JavaScript SDK
https://solana-labs.github.io/solana-web3.js
MIT License
1.97k stars 792 forks source link

feat: add isPublicKey with test #2826

Closed aspnxdd closed 2 weeks ago

aspnxdd commented 2 weeks ago

This PR aims to add a new functionality to validate if a PublicKeyInitDatacan actually construct a PublicKey

changeset-bot[bot] commented 2 weeks ago

⚠️ No Changeset found

Latest commit: 25766a4ad3aaa178f54ee90f73a5297e73a1f435

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

aspnxdd commented 2 weeks ago

Thanks for the submission!

Can you talk a little bit about what this new static method adds over this?

try {
  // Construct a new public key from some `initData`
  new PublicKey(initData);
} catch (e) {
  // Run this code when construction fails.
}

Your new static method calls the constructor, and the constructor() function already:

  • decodes base58 strings, throwing if they are invalid
  • asserts that they are 32 bytes, decoded

Your new method won't actually work for some public keys, because base58 encoded public keys can be anywhere between 32 and 44 characters.

Thanks for your reply. It is just a handy method, I personally usually build this function in every project.But I think it can actually be simplified to your suggestion, or I could add that the following instead:

try {
   new PublicKey(value);
   return true;
}
catch() { 
   return false
}

I just find it handy to have it as a static method in this class, in many projects I have to build this myself as a util, and I think it can help a bit. For example in JS you have the Array.isArray() or in etherjs you have Signer.isSigner() (source)

steveluscher commented 2 weeks ago

The difference with Array.isArray() is that you get it for free – it's built into the JS runtime. PublicKey already has too many methods on it that your application may not use but has to download anyway. Adding another punishes every application that doesn't need isPublicKey.

We're changing this architecture with the new 2.0 line of web3.js. I suggest that you take a look, and kick the tires if you have time! It won't be long before we recommend it for general use.

https://github.com/solana-labs/solana-web3.js/releases/tag/tp3

github-actions[bot] commented 4 days ago

Because there has been no activity on this PR for 14 days since it was merged, it has been automatically locked. Please open a new issue if it requires a follow up.