fix: TransactionHeader serialization

[juan518munoz]

Debugging #1691 with the miden client, found the following errors:

The TransactionHeader type changes from miden-base #1973 introduce a serialization mismatch in TransactionSummaryRowInsert::new.

The read side (TransactionRecordRaw::try_into) deserializes stored bytes as Vec<Nullifier> and Vec<NoteId>, but after the base update:

• transaction_header.input_notes() returns InputNotes<InputNoteCommitment> (was Vec<Nullifier>), where InputNoteCommitment includes an Option<NoteHeader> alongside the nullifier.
• transaction_header.output_notes() returns &[NoteHeader] (was Vec<NoteId>), where NoteHeader includes NoteMetadata alongside the note ID.

Calling .to_bytes() on these richer types produces a different binary format than what the read side expects, so we need to extract just the nullifiers and note IDs before serializing.

Cursor/push logic

Old code accumulated transactions in the loop but only stored the last one in last_added_tx. After the loop, it pushed that single transaction and updated the cursor. This was a bug — only the last transaction from each chunk was actually pushed to all_transactions, thus all earlier transactions in the chunk were silently dropped.

Fix pushes each transaction and updates the cursor immediately inside the loop, so every transaction that fits within the size limit is correctly added to all_transactions.

[mmagician]

LGTM, let's maybe add a roundtrip test for serialization here?

[drahnr]

This also needs a migration which is about to be introduced in #1700 , or do we never query the TX header from DB?

[juan518munoz]

LGTM, let's maybe add a roundtrip test for serialization here?

Added here

[juan518munoz]

This also needs a migration which is about to be introduced in #1700 , or do we never query the TX header from DB?

There's no schema change so we don't need to wait for #1700

[igamigo]

LGTM in terms of the code actually fixing the issue. However, we still cannot reconstruct the transaction header AFAIK. Not sure if this is something we want (I think it was discussed it at some point, but couldn't find much after a quick search cc @PhilippGackstatter), but I don't think we need it so this PR should be fine to merge.

[bobbinth]

Looks good! Thank you!

However, we still cannot reconstruct the transaction header AFAIK. Not sure if this is something we want (I think it was discussed it at some point, but couldn't find much after a quick search cc @PhilippGackstatter), but I don't think we need it so this PR should be fine to merge.

I thought we had an issue about this somewhere (to try to make transaction header consistent between protocol and protobuf implementations). If not, it may make sense to create one.

[bobbinth]

Question: why did we remove this section (and also the entry on line 26 above)?

[juan518munoz]

It was mistakenly removed, fixed!

[Mirko-von-Leipzig]

This also needs a migration which is about to be introduced in #1700 , or do we never query the TX header from DB?

There's no schema change so we don't need to wait for #1700

Just so we're on the same page here -- this, and the protocol migration, are not intended for v0.13 correct? If they are, then this becomes a much bigger lift.

[bobbinth]

Just so we're on the same page here -- this, and the protocol migration, are not intended for v0.13 correct? If they are, then this becomes a much bigger lift.

No - this should go into next with the intent to be deployed to devnet around Wednesday this week.

[drahnr]

nit: I'd appreciate backticks for types in comments

[drahnr]

Suggested change

	/// Converts the `ValidatorConfig` into a URL and an optional [`SocketAddr`].
	/// Converts the [`crate::commands::ValidatorConfig`] into a URL and an optional [`SocketAddr`].

[juan518munoz]

Linting was not part of actual code changes, as of rebasing into next, suggestion no longer applies.

[drahnr]

Bar the removal of SyncChainMmr from the docs and some nits, LGTM

[mmagician]

lgtm, but this should go to next as the bug is already there and not on the migration branch

[juan518munoz]

I thought we had an issue about this somewhere (to try to make transaction header consistent between protocol and protobuf implementations). If not, it may make sense to create one.

@bobbinth I believe you were refering to #1605

[igamigo]

Thanks!