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

fetch all messages and messages by owner #536

Merged
merged 23 commits into from
Aug 9, 2022

Conversation

leviathanbeak
Copy link
Contributor

@leviathanbeak leviathanbeak commented Aug 5, 2022

WIP

solves #521

DONE:

  • I have added all_owners_and_message_ids() to the Database
  • to the Schema I have added MessageQuery that includes messages() and messages_by_owner()
  • added a test but couldn't complete it since fuel-client needs to expose those APIs

TODO

  • expose messages() and messages_by_owner() APIs within fuel-client
  • finish the test in fuel-tests/tests/messages

@leviathanbeak leviathanbeak self-assigned this Aug 5, 2022
@leviathanbeak leviathanbeak marked this pull request as ready for review August 8, 2022 15:16
@leviathanbeak
Copy link
Contributor Author

wrote the test and exposed the respective APIs.

thanks to @bvrooman & @Voxelot for fixing and cleaning up the code.

@bvrooman
Copy link
Contributor

bvrooman commented Aug 8, 2022

This could just be me, but when I run the new test under fuel-tests/tests/messages.rs, I get the following message:

thread 'messages::messages' panicked at 'get_validators_da_height database corruption, err:Codec', fuel-core/src/database.rs:412:17
...

However, this message isn't exactly an error: the test execution still runs and the the test still passes.

It appears to be something to do with using bincode to deserialize the metadata for VALIDATORS_DA_HEIGHT_KEY:

// fuel-core/src/database/metadata.rs
self.insert(VALIDATORS_DA_HEIGHT_KEY, METADATA, 0)?;
..

// fuel-core/src/database.rs
// self.get calls bincode::deserialize
match self.get(metadata::VALIDATORS_DA_HEIGHT_KEY, METADATA) {
    ..
    Err(err) => {
        panic!(
            "get_validators_da_height database corruption, err:{:?}",
            err
        );
    }
}

Is anyone else seeing this?

@Voxelot
Copy link
Member

Voxelot commented Aug 8, 2022

Yeah this is a bug in the relayer code, i disabled the relayer by default in @ControlCplusControlV's branch

@Voxelot
Copy link
Member

Voxelot commented Aug 8, 2022

I'm thinking we could consolidate these two pagination endpoints by providing an optional MessageFilter object similar to what the coins API uses. I'll make a PR to this PR with the suggested changes.

@Voxelot
Copy link
Member

Voxelot commented Aug 9, 2022

here are my proposed changes to consolidate the two interfaces. When no owner is passed, it behaves normally as if iterating over the entire set of messages. When an owner is supplied to the API, the messages will be filtered based on the owner.

#540

@leviathanbeak leviathanbeak merged commit e5c7947 into master Aug 9, 2022
@leviathanbeak leviathanbeak deleted the leviathanbeak/fetch_all_messages branch August 9, 2022 15:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants