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

Remove deprecated API of bincode module. #32

Closed
wants to merge 1 commit into from
Closed

Remove deprecated API of bincode module. #32

wants to merge 1 commit into from

Conversation

vincenzopalazzo
Copy link

This PR is an API migration from a deprecated API that is bincode::config() to
new API that is bincode::option()

The following warning is removed during the building operation

warning: use of deprecated function `bincode::config`: please use `options()` instead
    --> src/new_index/schema.rs:1442:9
     |
1442 |         bincode::config()
     |         ^^^^^^^^^^^^^^^
     |
     = note: `#[warn(deprecated)]` on by default

warning: use of deprecated function `bincode::config`: please use `options()` instead
    --> src/new_index/schema.rs:1450:18
     |
1450 |             key: bincode::config().big_endian().serialize(&self.key).unwrap(),
     |                  ^^^^^^^^^^^^^^^

warning: use of deprecated function `bincode::config`: please use `options()` instead
    --> src/new_index/schema.rs:1456:19
     |
1456 |         let key = bincode::config()
     |                   ^^^^^^^^^^^^^^^

warning: 3 warnings emitted

I hope this can be helpful

@shesek
Copy link
Collaborator

shesek commented Feb 23, 2021

Hi @vincenzopalazzo, thanks for the PR!

We actually did this before and ended up reverting it because it was causing some issues. We should update the code to use the new bincode API, but it requires doing some more testing first.

@vincenzopalazzo
Copy link
Author

Hi @shesek,

I got it, I'm waiting for the electrics indexing on my laptop, maybe I can try to reproduce the bug and try to make some test on it.

If you agree, is it possible to move this PR as a draft? or you preferer to close it?

@shesek
Copy link
Collaborator

shesek commented Feb 23, 2021

It would be great if you tried to reproduce it, thanks!

It's possible that this issue only affects existing databases that were populated using bincode::config() and then read from using bincode::options(). Seeing if this works with a fresh install could help determine if that's the case or not.

@vincenzopalazzo
Copy link
Author

@shesek,

I got it, thanks, I will restart the sync on the weekend because I had some full memory during my work 😄 .

I hope to return here with a good result

This commit is an API migration from a deprecated API that is bincode::config() to
new API that is bincode::option()

The following warning are removed during the building operation

```bash
warning: use of deprecated function `bincode::config`: please use `options()` instead
    --> src/new_index/schema.rs:1442:9
     |
1442 |         bincode::config()
     |         ^^^^^^^^^^^^^^^
     |
     = note: `#[warn(deprecated)]` on by default

warning: use of deprecated function `bincode::config`: please use `options()` instead
    --> src/new_index/schema.rs:1450:18
     |
1450 |             key: bincode::config().big_endian().serialize(&self.key).unwrap(),
     |                  ^^^^^^^^^^^^^^^

warning: use of deprecated function `bincode::config`: please use `options()` instead
    --> src/new_index/schema.rs:1456:19
     |
1456 |         let key = bincode::config()
     |                   ^^^^^^^^^^^^^^^

warning: 3 warnings emitted
```

Signed-off-by: Vincenzo Palazzo <[email protected]>
junderw pushed a commit to junderw/electrs that referenced this pull request Sep 15, 2023
…flow

fix missing prevout tx fee underflow
@junderw
Copy link

junderw commented Sep 15, 2023

Sorry, I pushed something to my personal fork and for some reason it took the PR references from the mempool repo and converted them to this repo and made a mention, bumping the issue. Looks like a bug with GitHub.

@junderw
Copy link

junderw commented Sep 15, 2023

@shesek We fixed this in mempool.

https://github.com/mempool/electrs/pull/34/files#diff-e039d452ce5039daf198e6c7a80c09121f093a7a869811de08361bc553b90577R10-R16

The issue was that the new defaults for the Options API uses VarInt and disallows trailing.

You can check the PR, but essentially:

/// This is the default settings for Options,
/// but all explicitly spelled out, except for endianness.
/// The following functions will add endianness.
#[inline]
fn options() -> impl Options {
    bincode::options()
        .with_fixint_encoding()
        .with_no_limit()
        .allow_trailing_bytes()
}

/// Adding the endian flag for big endian
#[inline]
fn big_endian() -> impl Options {
    options().with_big_endian()
}

/// Adding the endian flag for little endian
#[inline]
fn little_endian() -> impl Options {
    options().with_little_endian()
}

@shesek
Copy link
Collaborator

shesek commented Feb 15, 2024

Thank you @junderw for pinning this down. I created a PR based on your suggestion and code at #70. Cheers :)

@shesek shesek closed this Feb 15, 2024
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.

3 participants