Skip to content
This repository has been archived by the owner on Jun 19, 2024. It is now read-only.

add memo and tx type indexing #97

Closed
wants to merge 9 commits into from

Conversation

dimiandre
Copy link
Contributor

add memo and tx type indexing for easy queries

@dimiandre
Copy link
Contributor Author

there is also a commit on vote_proposal i forgot in previous pr, sorry!

@rllola
Copy link
Contributor

rllola commented Feb 5, 2024

I need to fix the tests before accepting your PR. Just to be sure we are not breaking more things.

@rllola
Copy link
Contributor

rllola commented Feb 6, 2024

The tests are fixed. You can rebase and I will run them on your PR.

src/database.rs Outdated
@@ -599,7 +601,9 @@ impl Database {
for t in txs.iter() {
let tx = Tx::try_from(t.as_slice()).map_err(|_| Error::InvalidTxData)?;

let mut code = Default::default();
let mut code: [u8; 32] = Default::default();
let mut code_type: String = "wrapper".to_string();
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is redundant with tx_type which already exist in the table. You want to indicate if the tx is decrypted or wrapper... etc, right ?

Copy link
Contributor Author

@dimiandre dimiandre Feb 7, 2024

Choose a reason for hiding this comment

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

I'd like to indicate what that decrypted tx actually contains

tx_transfer
tx_ibc
etc

otherwise there is no easy way to "decode" code and query it directly from the database

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah ok. We use the code to indicate the tx_type but indeed you need to know it.
So just change the default value wrapper with unknown. Wrapper is another type (decrypted, wrapper, protocol,...) and it is different from what you want (aka tx_transfer, tx_ibc, ...). Just so we don't get mixed up.

@rllola
Copy link
Contributor

rllola commented Feb 19, 2024

Hi @dimiandre

If you want me to merge this, could you replace "wrapper" with "unknown" ? I think others will find it useful.

@dimiandre dimiandre requested a review from rllola February 21, 2024 09:06
@rllola
Copy link
Contributor

rllola commented Mar 11, 2024

Hey @dimiandre

I am sorry. There is some cnflicts do you think you can solve them ? I can't merge it

@rllola rllola closed this Apr 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants