feat(pubkeys): add getPubkeyBytes binding

[spiral-ladder]

Expose the cached 48-byte compressed pubkey bytes by validator index without materializing a PublicKey wrapper or serializing in JS. The BLS worker job path in Lodestar needs Uint8Array pubkeys, so this avoids the getOrThrow() + toBytes() round trip on the SingleAttestation gossip validation hot path.

Refs ChainSafe/lodestar#9728 (comment)

[twoeths]

how about

Suggested change

	return try js.Uint8Array.fromExternal(pubkey_bytes[0..]);
	return try js.Uint8Array.from(pubkey_bytes[0..]);

usually at lodestar side, the returned Uint8Array is a short-lived object that gets "gc" before we clone it via bls worker

[spiral-ladder]

makes sense

[spiral-ladder]

addressed in 5071c7c

[twoeths]

the general concern/question for me is when/how to use fromExternal() vs from()
given the current fromExternal() implementation in https://github.com/ChainSafe/zapi/blob/d6b21e193c9230d8da5fe4453331acfaff94396e/src/js/typed_arrays.zig#L75

pub fn fromExternal(slice: []const Element) !Self {
            const e = context.env();
            const buf = try context.allocator().dupe(Element, slice);

            const byte_len = slice.len * @sizeOf(Element);
            const len_hint: ?*anyopaque = @ptrFromInt(slice.len);
            const finalize_cb = comptime napi.wrapSliceFinalizeCallback(Element, externalFinalizer);
            const arraybuffer = e.createExternalArrayBuffer(std.mem.sliceAsBytes(buf), finalize_cb, len_hint) catch |err| {
                context.allocator().free(buf);
                return err;
            };

            _ = try e.adjustExternalMemory(@intCast(byte_len));
            const val = try e.createTypedarray(array_type, slice.len, arraybuffer, 0);
            return .{ .val = val };
        }

here we always alloc a memory at native there and count on a finalizer to free it
if we keep this fromExternal() implementation, we'll most likely to always go with from() where we let v8 alloc a memory and manage gc for us

[GrapeBaBa]

the general concern/question for me is when/how to use fromExternal() vs from() given the current fromExternal() implementation in https://github.com/ChainSafe/zapi/blob/d6b21e193c9230d8da5fe4453331acfaff94396e/src/js/typed_arrays.zig#L75

pub fn fromExternal(slice: []const Element) !Self {
            const e = context.env();
            const buf = try context.allocator().dupe(Element, slice);

            const byte_len = slice.len * @sizeOf(Element);
            const len_hint: ?*anyopaque = @ptrFromInt(slice.len);
            const finalize_cb = comptime napi.wrapSliceFinalizeCallback(Element, externalFinalizer);
            const arraybuffer = e.createExternalArrayBuffer(std.mem.sliceAsBytes(buf), finalize_cb, len_hint) catch |err| {
                context.allocator().free(buf);
                return err;
            };

            _ = try e.adjustExternalMemory(@intCast(byte_len));
            const val = try e.createTypedarray(array_type, slice.len, arraybuffer, 0);
            return .{ .val = val };
        }

here we always alloc a memory at native there and count on a finalizer to free it if we keep this fromExternal() implementation, we'll most likely to always go with from() where we let v8 alloc a memory and manage gc for us

In previous PR, GC pressure metrics observed and rust binding used external should be the two causes to switch to external. There is an ongoing PR to add a zero-copy API like napi-rs but it can't be used here as the ownership belong to pubkeys cache. May be still worth to observe the metrics.

[spiral-ladder]

@GrapeBaBa i have the same concern with @twoeths especially with adding yet another api. hence my comment in that PR. I think if that approach is strictly better we should just replace fromExternal with that new zero-copy version

[nazarhussain]

LGTM, but if we can add a test specific for failure case of this function that will be very helpful.

[GrapeBaBa]

@GrapeBaBa i have the same concern with @twoeths especially with adding yet another api. hence my comment in that PR. I think if that approach is strictly better we should just replace fromExternal with that new zero-copy version

Yes, zero-copy version can avoid copying for this kinds of code, but should we revisit these codes really need external array buffer?