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

Store data as raw bytes - txo - key_image (RPS-8) #851

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

briancorbin
Copy link
Contributor

@briancorbin briancorbin commented May 3, 2023

Removing some unnecessary use of protobuf encoding of raw bytes for fields in the txo object.

Future PR will include updates to the API, but that will be a breaking change

@briancorbin briancorbin changed the title Store data as raw bytes - txo - key_image Store data as raw bytes - txo - key_image (RPS-8) May 3, 2023
@briancorbin briancorbin force-pushed the store-data-as-raw-bytes/txo/key_image branch from 9a5b045 to da5cba5 Compare June 27, 2023 19:08
@briancorbin briancorbin force-pushed the store-data-as-raw-bytes/txo/target_key branch from ebe559b to 902c1d3 Compare June 27, 2023 19:08
@briancorbin briancorbin force-pushed the store-data-as-raw-bytes/txo/key_image branch from da5cba5 to 45f45e8 Compare December 5, 2023 16:57
@briancorbin briancorbin changed the base branch from store-data-as-raw-bytes/txo/target_key to main December 5, 2023 16:58
@briancorbin briancorbin marked this pull request as ready for review December 5, 2023 17:26
Copy link
Contributor

@eranrund eranrund left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This appears to be a very breaking change since it alters the format inside the database. Whats the migration plan for it?

@@ -3,6 +3,8 @@
//! A subaddress assigned to a particular contact for the purpose of tracking
//! funds received from that contact.

use std::convert::TryInto;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: no new line, all imports grouped together

@@ -170,6 +170,9 @@ pub enum WalletDbError {

/// MemoDecoding: {0}
MemoDecoding(MemoDecodingError),

/// Ring Signature error: {0}
RingSignature(mc_transaction_core::ring_signature::Error),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious why this was needed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants