wasm vs asm returns different results

[kvhnuke]

import { asmJsInit } from "@polkadot/wasm-crypto-asmjs/data";
import { wasmBytes } from "@polkadot/wasm-crypto-wasm";
import { initWasm } from "@polkadot/wasm-crypto/bridge";
import { secp256k1FromSeed } from "@polkadot/wasm-crypto";
import * as imports from "@polkadot/wasm-crypto/imports";
const wasmPromise = initWasm(wasmBytes, asmJsInit, imports).catch(() => null);
wasmPromise.then(() => {
  console.log(
    secp256k1FromSeed([
      203,
      109,
      249,
      222,
      30,
      252,
      167,
      163,
      153,
      138,
      142,
      173,
      78,
      2,
      21,
      157,
      95,
      169,
      156,
      62,
      13,
      79,
      214,
      67,
      38,
      103,
      57,
      11,
      180,
      114,
      104,
      84,
    ])
  );
});

outputs

[
  203, 109, 249, 222,  30, 252, 167, 163, 153, 138, 142,
  173,  78,   2,  21, 157,  95, 169, 156,  62,  13,  79,
  214,  67,  38, 103,  57,  11, 180, 114, 104,  84,   2,
   10,  16, 145,  52,  31, 229, 102,  75, 250,  23, 130,
  213, 224,  71, 121, 104, 144, 104, 201,  22, 176,  76,
  179, 101, 236,  49,  83, 117,  86, 132, 217, 161
]

however if you just use asm,

const wasmPromise = initWasm([], asmJsInit, imports).catch(() => null);

itll output

[
  203, 109, 249, 222,  30, 252, 167, 163, 153, 138, 142, 173,
   78,   2,  21, 157,  95, 169, 156,  62,  13,  79, 214,  67,
   38, 103,  57,  11, 180, 114, 104,  84,   2,   0,   0,   0,
    0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,
    0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,
    0,   0,   0,   0,   0
]

seems like asm is somehow broken

[jacogr]

That would be slightly problematic to say the least since it is just a big black compilation box.

[kvhnuke]

also yarn test doesnt seem to cover the test:wasm:js

[jacogr]

Indeed. The steps are a bit counter-intuitive, the build step actually runs the wasm/asm tests since it needs to have the compiled outputs.

[kvhnuke]

ah I see, thanks
also is there a reason for just have asm for react-native https://github.com/polkadot-js/wasm/blob/master/packages/wasm-crypto-asmjs/package.json#L23

chrome extension with latest manifest v3 doesnt allow webassembly (smh, the new manifest is a mess)

[jacogr]

The issue with the asm.js is that it is exceptionally large, so it really bloats everything when compiled into a web bundle. On the RN side there is no WASM support, so it is only enabled there.

(It is quite a dance atm to swap between the two, but I don't have good ideas to make it "simple" - maybe the approach would be to have an wasm, asm and both of path exposed for easy access - but not sure how to get past init then either)

I saw the manifest 3 thing, it was logged in the extension repo, it is a bridge I need to cross soon as well since the time is ticking.

I can reproduce your issue and am looking at it. Weirdly it doesn't happen in #308 (and I did indeed check that the aliasing works correctly, i.e. that it indeed follows the asm path. So trying to figure out why there is a discrepancy between the two.)

[jacogr]

Ok... this is weird - I can reproduce it in the repo if I comment out all the other JS tests and only run the secp256k1 tests, specifically the 2 seed tests. If I add some more tests, it passes.

Not that this helps, but thinking out loud -

• something in the asm code is not immediate initialized
• something in the secp code needs "some" time

At least can now reproduce it in here, so that is a step in the right direction.

Update: If I remove only the scrypt test, the seed tests on secp256k1 fails (well, the first one fails), this line -

wasm/packages/wasm-crypto/test/all/hashing.cjs

Line 95 in 63585ff

scryptHash,

Update 2: Linked a PR that moves the secp256k1 tests first, which reproduced this on CI.

[kvhnuke]

oh wow never thought of that, had to go sleep after trying to debug for 3 hrs
literally adding a setTimeout(()=>{secp256k1FromSeed([...])},100) fixes it.
yea Im speechless too

[jacogr]

Would try to upgrade the wasm build deps - if an issue there, it is quite a bit behind so maybe there is a fix in there somewhere.

I am guessing it is the lazy sections in the secp256k1.

Worst case, will swap the Rust crate to the C++ wrapper. Either way, this is not great to say the least.

[jacogr]

All the WASM generator deps have been bumped. This had no effect.

This HACK does yield the right results, which I'm certainly not happy with, but I have no other levers to pull here - e840a6f (Or rather, no levers that I could find, maybe the wasm2js has an additional cli option that can help?)

wasm/packages/wasm-crypto/src/bundle.ts

Lines 57 to 72 in 0dfb491

	let secpLazyInit = false;
	export const secp256k1FromSeed = withWasm((wasm, seckey: Uint8Array): Uint8Array => {
	// HACK In an asm.js environment, the static initialization seems to trip up the
	// first usage of the keypair generation
	// https://github.com/polkadot-js/wasm/issues/307
	if (!secpLazyInit && __bridge.type === 'asm') {
	wasm.ext_secp_from_seed(8, ...allocU8a(seckey));
	resultU8a();
	}
	secpLazyInit = true;
	wasm.ext_secp_from_seed(8, ...allocU8a(seckey));
	return resultU8a();
	});

Additionally going to add an assert in @polkadot/util-crypto to ensure that all-zero public keys are not returned. It went in here - https://github.com/polkadot-js/common/blob/2563a0ac3ddfc8de17f907861aa455b3d8db6b49/packages/util-crypto/src/secp256k1/pair/fromSeed.ts#L18-L32

[jacogr]

As an aside ...

In the 5.0.1 release, the init would be simpler for asm-only or asm + wasm (intstead of duplicating what is done internally, this would hopefully make is easier for people to adjust) -

// ... or initWasmAsm to have both asm & WASM support
import "@polkadot/wasm-crypto/initOnlyAsm";

import { secp256k1FromSeed, waitReady } from "@polkadot/wasm-crypto";

waitReady().then(() => {
	console.log(
		secp256k1FromSeed(new Uint8Array([
			203, 109, 249, 222, 30, 252, 167, 163, 153, 138, 142,
			173, 78, 2, 21, 157, 95, 169, 156, 62, 13, 79,
			214, 67, 38, 103, 57, 11, 180, 114, 104, 84
		]))
	);
});

[kvhnuke]

As an aside ...

In the 5.0.1 release, the init would be simpler for asm-only or asm + wasm (intstead of duplicating what is done internally, this would hopefully make is easier for people to adjust) -

// ... or initWasmAsm to have both asm & WASM support
import "@polkadot/wasm-crypto/initOnlyAsm";

import { secp256k1FromSeed, waitReady } from "@polkadot/wasm-crypto";

waitReady().then(() => {
	console.log(
		secp256k1FromSeed(new Uint8Array([
			203, 109, 249, 222, 30, 252, 167, 163, 153, 138, 142,
			173, 78, 2, 21, 157, 95, 169, 156, 62, 13, 79,
			214, 67, 38, 103, 57, 11, 180, 114, 104, 84
		]))
	);
});

this is perfect for my use case, thanks @jacogr

[jacogr]

The need for the hack disappears in #318 - which is great news.

The above swaps to the Bitcoin secp256k1 library, which also aligns with the use in Substrate and as a bonus is more performant.

[polkadot-js-bot]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue if you think you have a related problem or query.