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

Use network when calculating difficulty #2168

Merged

Conversation

tcharding
Copy link
Member

@tcharding tcharding commented Nov 3, 2023

The difficulty is a ratio of the max and current targets, since the max is network specific the difficulty calculation is also network specific.

We already have network specific maximum target constants, use them when calculating the difficulty.

Patch 1 is a trival docs improvement to block::Header::difficulty.

Copy a sentence from the `pow::Target::difficulty` function onto the
`block:Header::difficulty` function.
The difficulty is a ratio  of the max and current targets, since the
max is network specific the difficulty calculation is also network
specific.

We already have network specific maximum target constants, use them when
calculating the difficulty.
@tcharding
Copy link
Member Author

Done as part of #2121

Copy link
Collaborator

@Kixunil Kixunil left a comment

Choose a reason for hiding this comment

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

ACK 12d615d

Network::Testnet => Target::MAX_ATTAINABLE_TESTNET,
Network::Signet => Target::MAX_ATTAINABLE_SIGNET,
Network::Regtest => Target::MAX_ATTAINABLE_REGTEST,
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

Feels like this should be in consensus params.

Copy link
Member Author

@tcharding tcharding Nov 3, 2023

Choose a reason for hiding this comment

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

These are in Params::pow_limit, I thought it better to take a Network parameter not a Params parameter though because its more specific. Or am I missing something?

Copy link
Member Author

Choose a reason for hiding this comment

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

We could have this?

    pub fn difficulty(&self, params: &Params) -> u128 {
        let max = params.pow_limit;
        let d = max.0 / self.0;
        d.saturating_to_u128()
    }

Copy link
Collaborator

Choose a reason for hiding this comment

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

I do think Network makes sense but I was thinking Params::new(network).pow_limit.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Once we have Network: AsRef<Params>/Network: AsParams we should just take generic.

@tcharding
Copy link
Member Author

I think this is right to go in, we can sort out the consts when doing #2169 and #2173

Copy link
Member

@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

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

ACK 12d615d

@apoelstra apoelstra merged commit 966b190 into rust-bitcoin:master Nov 6, 2023
29 checks passed
tcharding added a commit to tcharding/rust-psbt that referenced this pull request Nov 10, 2023
Import code from https://github.com/rust-bitcoin/rust-bitcoin` comit:
`966b190f238408e24efd768ffa2bcdb6046e0cea Merge
rust-bitcoin/rust-bitcoin#2168: Use network when calculating difficulty`

Also grab required code from `bitcoin/src/serde_utils.rs` and
`internals/src/error.rs`.

Make changes to get things building:

- Use the most terse public bitcoin API (ie, don't reach down into
  modules when importing if there is a re-export at crate level)
- Fix paths to use correct `crate::` vs `bitcoin::`
- Import `bitcoin::consensus::encode` as `consensus` and add TODO
  comments about it.
tcharding added a commit to tcharding/rust-psbt that referenced this pull request Nov 10, 2023
Import code from https://github.com/rust-bitcoin/rust-bitcoin` comit:
`966b190f238408e24efd768ffa2bcdb6046e0cea Merge
rust-bitcoin/rust-bitcoin#2168: Use network when calculating difficulty`

Also grab required code from `bitcoin/src/serde_utils.rs` and
`internals/src/error.rs`.

Make changes to get things building:

- Use the most terse public bitcoin API (ie, don't reach down into
  modules when importing if there is a re-export at crate level)
- Fix paths to use correct `crate::` vs `bitcoin::`
- Import `bitcoin::consensus::encode` as `consensus` and add TODO
  comments about it.
tcharding added a commit to tcharding/rust-psbt that referenced this pull request Nov 12, 2023
Import code from https://github.com/rust-bitcoin/rust-bitcoin` comit:
`966b190f238408e24efd768ffa2bcdb6046e0cea Merge
rust-bitcoin/rust-bitcoin#2168: Use network when calculating difficulty`

Also grab required code from `bitcoin/src/serde_utils.rs` and
`internals/src/error.rs`.

Make changes to get things building:

- Use the most terse public bitcoin API (ie, don't reach down into
  modules when importing if there is a re-export at crate level)
- Fix paths to use correct `crate::` vs `bitcoin::`
- Import `bitcoin::consensus::encode` as `consensus` and add TODO
  comments about it.
lucky-whistle-dev added a commit to lucky-whistle-dev/rust-bsbt that referenced this pull request Apr 1, 2024
Import code from https://github.com/rust-bitcoin/rust-bitcoin` comit:
`966b190f238408e24efd768ffa2bcdb6046e0cea Merge
rust-bitcoin/rust-bitcoin#2168: Use network when calculating difficulty`

Also grab required code from `bitcoin/src/serde_utils.rs` and
`internals/src/error.rs`.

Make changes to get things building:

- Use the most terse public bitcoin API (ie, don't reach down into
  modules when importing if there is a re-export at crate level)
- Fix paths to use correct `crate::` vs `bitcoin::`
- Import `bitcoin::consensus::encode` as `consensus` and add TODO
  comments about it.
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.

None yet

4 participants