simplicity icon indicating copy to clipboard operation
simplicity copied to clipboard

rawElementsInput.annex ought not to be a pointer

Open apoelstra opened this issue 3 months ago • 2 comments

In the rawElementsInput struct we have an annex field whose type is const rawElementsBuffer*. But rawElementsBuffer is already a pointer type. Every other instance of rawElementsBuffer is used directly, rather than through a const pointer.

In Elements interpreter.cpp this requires allocating a separate vector simplicityRawAnnex. In Elements we have some assurance that none of the data structures passed to Simplicity will outlive this vector, but it's hard to make this assurance in the general-purpose Rust library. (Our current code has had a TODO for years to support the annex field, which is how we didn't notice this awkwardness.)

I propose replacing the pointer with an inline buffer, as is done for scriptPubKey,the rangeproof and surjection proof, etc.

apoelstra avatar Sep 04 '25 22:09 apoelstra

It is a pointer in order to simulate an option type, since the null annex is different from the empty annex.

We could make it inline and add a bool flag. It's unclear to me if such a change is for the better or for the worse.

roconnor-blockstream avatar Sep 05 '25 19:09 roconnor-blockstream

Ah, ok, makes sense. Personally I'd prefer having a separate boolean -- or a nullable pointer and an explicit length, if you don't want to introduce new fields.

apoelstra avatar Sep 05 '25 19:09 apoelstra