Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat!: completed transaction use bytes for transaction protocol (not hex string) in wallet database #5906

Merged

Conversation

hansieodendaal
Copy link
Contributor

@hansieodendaal hansieodendaal commented Nov 4, 2023

Description

Changed the transaction protocol in the wallet database to use bytes instead of a hex string, as the underlying data type is encrypted bytes. This issue was highlighted due to the to_hex function in tari_utilities not being able to convert large transactions into hex strings (returned **String to large**) for saving in the wallet database during system-level coin-split stress testing.

Motivation and Context

See above.

How Has This Been Tested?

Existing unit tests and cucumber tests passed
Added two new unit tests and a cucumber test (long-running)
System-level test creating a coin-split transaction with 499 outputs passed

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

Code walk-through
Run the new unit tests
Perform a system-level test creating a coin-split transaction with 499 outputs and wait for it to be mined and confirmed,

Breaking Changes

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

BREAKING CHANGE: All completed transactions in the wallet database will be deleted at startup, while subsequent output validation will re-allocate outputs that were encumbered to be spent as available and invalidate outputs that were encumbered to be received.

Changed the transaction protocol in the wallet database to use bytes instead of
a hex string, as the underlying data type is encrypted bytes. This issue was
highlighted due to the `to_hex` function in `tari_utilities` not being able
to convert large transactions into hex strings (`**String to large**`) during
system-level coin-split stress testing.
@ghpbot-tari-project ghpbot-tari-project added P-acks_required Process - Requires more ACKs or utACKs P-reviews_required Process - Requires a review from a lead maintainer to be merged labels Nov 4, 2023
Copy link

github-actions bot commented Nov 4, 2023

Test Results (CI)

1 242 tests   1 242 ✔️  9m 50s ⏱️
     39 suites         0 💤
       1 files           0

Results for commit 92e37e1.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Nov 4, 2023

Test Results (Integration tests)

  2 files  +  2  11 suites  +11   24m 10s ⏱️ + 24m 10s
31 tests +31  29 ✔️ +29  0 💤 ±0  2 +2 
33 runs  +33  31 ✔️ +31  0 💤 ±0  2 +2 

For more details on these failures, see this check.

Results for commit 92e37e1. ± Comparison against base commit 1195afb.

♻️ This comment has been updated with latest results.

@hansieodendaal hansieodendaal changed the title [wip] feat!: change transaction protocol to use bytes rather than a hex string in the wallet database feat!: change transaction protocol to use bytes rather than a hex string in the wallet database Nov 4, 2023
@hansieodendaal hansieodendaal changed the title feat!: change transaction protocol to use bytes rather than a hex string in the wallet database feat!: change completed transaction to use bytes for transaction protocol rather than a hex string in the wallet database Nov 4, 2023
@hansieodendaal hansieodendaal changed the title feat!: change completed transaction to use bytes for transaction protocol rather than a hex string in the wallet database feat!: completed transaction use bytes for transaction protocol rather than hex string in wallet database Nov 4, 2023
@hansieodendaal hansieodendaal changed the title feat!: completed transaction use bytes for transaction protocol rather than hex string in wallet database feat!: completed transaction use bytes for transaction protocol (not hex string) in wallet database Nov 4, 2023
@ghpbot-tari-project ghpbot-tari-project removed the P-reviews_required Process - Requires a review from a lead maintainer to be merged label Nov 6, 2023
@SWvheerden SWvheerden merged commit 61256cd into tari-project:development Nov 6, 2023
16 of 20 checks passed
@hansieodendaal hansieodendaal deleted the ho_transaction_too_large branch November 6, 2023 09:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P-acks_required Process - Requires more ACKs or utACKs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants