Should we make FIELD_ELEMENTS_PER_BLOB runtime-configurable? about c-kzg-4844 HOT 8 CLOSED

In C, you will be able to allocate a blob on the stack with the new definition like this:

uint8_t blob[s->bytes_per_blob];

But I'm not a huge fan of this and I think it might have some portability issues.

Don't do this. Not only does this indeed have portability issues: Automatic storage duration (i.e. in practise stack-allocated) VLAs are optional even in C23 (Note VLAs as types are fine, e.g. as argument types. This still can take malloc-allocated or fixed-size array from callers).
Furthermore, the behaviour/error handling when running out of stack-memory is generally not robust: You cannot portably guard against this beforehand, I don't think you can catch it and if you are unlucky, it's a security issue (if mem_needed > stack_availiable + guard_page_size, assume bad things may happen unless you can prove otherwise).

from c-kzg-4844.

For Teku, it will simplify the java bindings (no need of two libs) and allows us to run the consensus reference tests without doing too many hacky changes. So overall I am happy with this change. Is there a possibility of doing another audit after this change?

from c-kzg-4844.

it would definitely be amazing for supporting minimal setups 👍

from c-kzg-4844.

Nim is flexible enough to handle multiple configurations thanks to it's powerful metaprogramming. But this changes clearly will make Nimbus code cleaner.

from c-kzg-4844.

Super helpful in lighthouse as well since we don't have to resort to importing the same library twice

from c-kzg-4844.

Minor: Reduce the amount of heavy 128kb blobs on the stack (our sizes are not a problem for modern computers)

Slightly more complicated memory management because of dynamic heap usage instead of static stack usage

I don't think these two sentences make sense. Whether the blobs are in the stack or the heap doesn't depend on the library definition, but depends on the library users and bindings. For example, if I have a definition of

typedef struct {
    uint8_t bytes[BYTES_PER_BLOB];
} Blob;

It doesn't mean immediately that the blobs will always be on the stack. If the user wants it to be on the stack, they can do so.

Blob blob;

If the user wants it to be on the heap, they can also do so.

Blob *blob = malloc(sizeof(Blob));

The same reason can be applied to the new blob definition. (uint8_t *blob)

from c-kzg-4844.

@ppopth you're right. With our current definition of Blob, you can allocate a blob on the stack or heap. I think this point stems from the Rust bindings, where the Blob definition being used (see below) is difficult to directly allocate on the heap. They needed to do something like this to accommodate.

#[repr(C)]
pub struct Blob {
    bytes: [u8; BYTES_PER_BLOB],
}

In C, you will be able to allocate a blob on the stack with the new definition like this:

uint8_t blob[s->bytes_per_blob];

But I'm not a huge fan of this and I think it might have some portability issues.

from c-kzg-4844.

This is no longer necessary. As of #377, mainnet & minimal use the same trusted setup.

from c-kzg-4844.