feat: optimize transaction validation db queries

[hansieodendaal]

Description

Optimized transaction validation SQLite queries to run in batch mode wherever possible to minimise db operations.

Motivation and Context

See #6195

How Has This Been Tested?

Updated unit tests
Added a raw query regression test
System-level testing

What process can a PR reviewer use to test or verify this change?

Review code and tests

Breaking Changes

• None
• Requires data directory on base node to be deleted
• Requires hard fork
• Other - Please specify

[github-actions]

Test Results (CI)

    3 files    120 suites   38m 37s ⏱️
1 269 tests 1 269 ✅ 0 💤 0 ❌
3 799 runs  3 799 ✅ 0 💤 0 ❌

Results for commit 4524995.

♻️ This comment has been updated with latest results.

[github-actions]

Test Results (Integration tests)

 2 files  11 suites   24m 32s ⏱️
32 tests 31 ✅ 0 💤 1 ❌
33 runs  32 ✅ 0 💤 1 ❌

For more details on these failures, see this check.

Results for commit 4524995.

♻️ This comment has been updated with latest results.

[SWvheerden]

This looks good, I just dont like the new trait function names for the reasons stated

[stringhandler]

While this SQL doesn't include a SQL Injection vulnerabilty, it is not using best practices for including parameters in sql. This should be changed now rather than later

[stringhandler]

Use bound parameters instead

[stringhandler]

Perhaps it's best to not update this if the last_validation_timestamp is newer than the current timestamp

[hansieodendaal]

I do not follow. How could last_validation_timestamp be newer than the current timestamp?

[stringhandler]

See above

[stringhandler]

This logic seems different. Why would we insert empty data? Is there a process to correct these amounts?

[hansieodendaal]

The first part of the query only identifies the rows that would be affected
INSERT INTO outputs (..) VALUES (...) ON CONFLICT (commitment) and does not change anything.
The only values in that list that are important are the ones to be used in the second part of the query, so all other values can by NULL / 0.

The actual update comes after as

DO UPDATE SET mined_height = excluded.mined_height, mined_in_block = \
    excluded.mined_in_block, status = excluded.status, mined_timestamp = excluded.mined_timestamp, \
    marked_deleted_at_height = NULL, marked_deleted_in_block = NULL, last_validation_timestamp = NULL

That is equivalent to

            .set((
                outputs::mined_height.eq(mined_height as i64),
                outputs::mined_in_block.eq(mined_in_block),
                outputs::status.eq(status),
                outputs::mined_timestamp.eq(timestamp),
                outputs::marked_deleted_at_height.eq::<Option<i64>>(None),
                outputs::marked_deleted_in_block.eq::<Option<Vec<u8>>>(None),
                outputs::last_validation_timestamp.eq::<Option<NaiveDateTime>>(None),

[brianp]

How often do these values change, and does the ordering here matter.

For maintenance overtime how easy is this for a developer to come along, remove or add a field and break the query.

Are these fields managed anywhere as a list in constants or fields of an existing structure we could reference instead of strings in the query?

[hansieodendaal]

That list comprises field names in diesel::table! {outputs (id) {...} } that cannot be left empty when inserting a row and all fields that need to be updated in the second half of the query if not included already. The frequency of change would coincide with changes to that table.

For future maintenance, I have added unit test pub async fn test_raw_custom_queries_regression(). It specifically tests the raw queries in set_received_outputs_mined_height_and_statuses and mark_outputs_as_spent.

As an example, executing the full query in DB Browser for SQLite for two rows:

INSERT INTO outputs 
(commitment, mined_height, mined_in_block, status, mined_timestamp, spending_key, value, output_type, maturity, hash, script, input_data, script_private_key, sender_offset_public_key,
metadata_signature_ephemeral_commitment, metadata_signature_ephemeral_pubkey, metadata_signature_u_a, metadata_signature_u_x, metadata_signature_u_y, spending_priority, covenant,
encrypted_data, minimum_value_promise) 
VALUES 
(x'be9a7ce5362db3882fdf138de9840a9329ae48e30c945d60a013758f76ce7239', 2, x'00', 6, '2024-03-01 11:52:54', 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0), 
(x'de0f779865d752563f609c07669afff260b946bb2c805b933c071c26e308c232', 2, x'00', 6, '2024-03-01 11:59:04', 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0) 
ON CONFLICT (commitment) DO UPDATE SET 
mined_height = excluded.mined_height, mined_in_block = excluded.mined_in_block, status = excluded.status, mined_timestamp = excluded.mined_timestamp, marked_deleted_at_height = NULL, marked_deleted_in_block = NULL, last_validation_timestamp = NULL

results in

Execution finished without errors.
Result: query executed successfully. Took 0ms, 2 rows affected

As a test let's leave out input_data from the list of columns as well as the corresponding 0 in both value rows, then we get

Execution finished with errors.
Result: NOT NULL constraint failed: outputs.input_data