[bug] finalize_transaction returns NULL if the final transaction size is greater than 1023 bytes

[raffecat]

Bug Report

Current behavior:

After adding 58 UTXOs via add_utxo, finalize_transaction returns NULL (we're trying to send a lot of Dogecoin.)

Upon investigation, it's because finalize_transaction calls get_raw_transaction, which then calls utils_uint8_to_hex, and that has an internal size limit of 1023 bytes (TO_UINT8_HEX_BUF_LEN/2-1). This is because it uses a static buffer buffer_uint8_to_hex with a fixed size of 2048 hex chars.

Also note that utils_uint8_to_hex returns a pointer to this static buffer, as does get_raw_transaction and finally finalize_transaction ends up returning this static buffer address out through the public API, which might be surprising.

Expected behavior

finalize_transaction should return the encoded hex form of the transaction, even for transactions larger than 1023 bytes (2046 hex chars.)

Steps to reproduce:

Test case: #261

However any transaction with more than ~25 UTXOs will hit the size limit.

libdogecoin commit

0.1.5-dev @ c8955d2, self-compiled

The same code is also present on 0.2.0-dev

Machine specs

• OS: Mac OS 12.7.6
• CPU: M1 Max
• RAM: 32 GB
• Disk size: ugh 2TB
• Disk Type (HD/SDD): SSD

Extra information

FAILED - test_transaction() - Line 540
Expect: not NULL
Receive: NULL

[edtubbs]

Addressed in #262 as the near term fix. finalize_transaction() still returns the shared static buffer pointer, which is fine for single threaded use. We’ll revisit dynamic allocation or a mutex for concurrency.

[raffecat]

@edtubbs thanks for the speedy response!

I feel I should raise this though - this is great for most use-cases, but it does increase the BSS size by 100K, which might be a problem for embedded use-cases, does this impact radiodoge?

[edtubbs]

Radiodoge runs libdogecoin only on the hub, so the 100 KB buffer is fine there. We can revisit if we ever target tiny microcontrollers directly.

[michilumin]

Ed's correct on this - RD would only encounter this code on the hub and; right now the hub runs on x86; but we have a plan to dynamically allocate anyhow. Right now the RD clients even are using R5s and T6es like dogebox which also have PC-like levels of memory anyhow; However I certainly acknowledge the need to dynamically allocate this down the line, and its definitely a solid to-do. This was just a fix to get it ameliorated quickly and have it align with core's size values.