web3.js: Deprecate async methods “recently” made sync

[steveluscher]

Problem

createProgramAddress and findProgramAddress are async methods, despite having no awaitables in their implementation. A developer using these methods will incorrectly presume that their code needs to be made async. This leads to a proliferation of async/await in the ecosystem which reduces the overall reliability and performance of apps.

Proposed Solution

Discourage people from using them by marking their method signatures as @deprecated.

Alternate solution

The only reason that these methods were made sync was because the last bit of async code was removed from createProgramAddress. That code was replaced with a sync version because it was incompatible in some way with React Native. cc/ @dr497

Instead, we might consider doubling down on restoring the former glory of the async version, by using an async hash method, then adding a React Native compatible hashing algorithm using the __forks__ directory. Last step would be to build a React Native bundle.

[tomland123]

I think the only person in the world who cares about these optimizations is me, but it's a bit irritating to watch thousands of hours of engineering work get forgotten and be replaced with a simpler abstraction that works.

I am fine with deprecating it. Shipping speed is more important than my bikeshedding

[steveluscher]

I care.

It's hard to know where the global maximum is here, while all of these things are true:

• 🚀 There exists the opportunity to produces hashes on multiple threads in Node.
• 🌍 That notwithstanding, JavaScript isn't multithreaded and the same browser APIs typically can't make use of multiple threads.
• 💤 async/await code is difficult to reason about and the degree to which folks in the ecosystem find it difficult to reason about is laying waste to the reliability and performance of ecosystem apps.

All of the asymmetry here either makes things malperformant, confusing, or complex. It's like an iron triangle but where each point is only bad things.

[jordaaash]

Woah, are we really importing the entire Ethers lib just to use that one utility function?

(asking because Ethers ships with small modular packages that we should very much use if possible)

Edit: this was apparently changed to only import the needed hash function:

solana/web3.js/package.json

Line 60 in 56d38e4

"@ethersproject/sha2": "^5.5.0",

[ftelnov]

Plus here, another reason to make them sync - imagine writing synchronous JS code and here you need to somehow include promise resolving. Strange thing.

Personally, I created simple replacement in my projects - just got sources and a bit rewrote them. Saved me a lot of possible rewritings :)

[steveluscher]

Woah, are we really importing the entire Ethers lib just to use that one utility function?

Yeah, the question is whether the ethers library is tree-shakable or not. I almost certainly investigated this in #25014, but I investigated a lot of library dependencies in that PR that I don't remember.

I'll get back to it, and get these dependencies under control.

[jordaaash]

Yeah, the question is whether the ethers library is tree-shakable or not.

As originally imported, it was grabbing ethers.utils.sha256 which imports the entire set of utils (including a bunch of other hash functions). But b825390 fixed this.

[steveluscher]

Yeah, that's never the whole story with tree-shaking though. For instance, the browser-sha2.ts file imports Logger which sha256() doesn't use. Whether Logger gets tree-shaken out is a function of whether or not:

• Ethers has marked their package as sideEffects: false (they have)
• @ethersproject/logger has marked their package as sideEffects: false (¯\_(°ペ)_/¯)
• Ethers is actually side-effect free (given that they call new Logger right in the module factory, I'm going to guess the answer is ‘no’)