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.

Sign up for GitHub

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

Jump to bottom

relative::LockTime should implement ordered::ArbitraryOrd #2566

Closed
apoelstra opened this issue Mar 10, 2024 · 3 comments · Fixed by #2581
Closed

relative::LockTime should implement ordered::ArbitraryOrd #2566

apoelstra opened this issue Mar 10, 2024 · 3 comments · Fixed by #2581
Milestone
0.32.0

Comments

@apoelstra
Copy link
Member

Maybe worth thinking of other types that should implement ArbitraryOrd as well.
@apoelstra apoelstra mentioned this issue Mar 10, 2024
Merged
@tcharding tcharding added this to the 0.32.0 milestone Mar 11, 2024
@15IITian
Copy link
Contributor

rust-bitcoin/bitcoin/src/blockdata/locktime/absolute.rs

Line 405 in 6f6cc00

impl ordered::ArbitraryOrd for LockTime {

why it has been disabled?

@tcharding
Copy link
Member

Its feature gated to give users control of their dependency graph, i.e., only enable whats needed.

tcharding added a commit to tcharding/rust-bitcoin that referenced this issue Mar 13, 2024
@tcharding
Implement ArbitraryOrd for relative::LockTime
2f0859c 
TL;DR As we do for `absolute::LockTime` and for the same reasons;
implement `ArbitraryOrd` for `relative::LockTime`.

locktimes do not have a semantic ordering if they differ (blocks, time)
so we do not derive `Ord` however it is useful for downstream to be able
to order structs that contain lock times. This is exactly what the
`ArbitraryOrd` trait is for.

Fix: rust-bitcoin#2566
@tcharding tcharding mentioned this issue Mar 13, 2024
Merged
@tcharding
Copy link
Member

Opened #2580 to track the "Maybe worth checking ..." bit - this issue will be closd by #2581

tcharding added a commit to tcharding/rust-bitcoin that referenced this issue Mar 13, 2024
@tcharding
Implement ArbitraryOrd for relative::LockTime
11867db 
TL;DR As we do for `absolute::LockTime` and for the same reasons;
implement `ArbitraryOrd` for `relative::LockTime`.

locktimes do not have a semantic ordering if they differ (blocks, time)
so we do not derive `Ord` however it is useful for downstream to be able
to order structs that contain lock times. This is exactly what the
`ArbitraryOrd` trait is for.

Fix: rust-bitcoin#2566
tcharding added a commit to tcharding/rust-bitcoin that referenced this issue Mar 20, 2024
@tcharding
Implement ArbitraryOrd for relative::LockTime
53c5d4f 
TL;DR As we do for `absolute::LockTime` and for the same reasons;
implement `ArbitraryOrd` for `relative::LockTime`.

locktimes do not have a semantic ordering if they differ (blocks, time)
so we do not derive `Ord` however it is useful for downstream to be able
to order structs that contain lock times. This is exactly what the
`ArbitraryOrd` trait is for.

Fix: rust-bitcoin#2566
tcharding added a commit to tcharding/rust-bitcoin that referenced this issue Mar 24, 2024
@tcharding
Implement ArbitraryOrd for relative::LockTime
f5841e3 
TL;DR As we do for `absolute::LockTime` and for the same reasons;
implement `ArbitraryOrd` for `relative::LockTime`.

locktimes do not have a semantic ordering if they differ (blocks, time)
so we do not derive `Ord` however it is useful for downstream to be able
to order structs that contain lock times. This is exactly what the
`ArbitraryOrd` trait is for.

Update the rustdocs in `relative` and mirror the docs changes in
`absolute`.

Fix: rust-bitcoin#2566
apoelstra added a commit that referenced this issue Apr 2, 2024
@apoelstra
Merge #2581: Implement ArbitraryOrd for relative::LockTime
6a2fd96 
d91cdd2 docs: Document ordered feature (Tobin C. Harding)
3520f55 Implement ArbitraryOrd for relative::LockTime (Tobin C. Harding)

Pull request description:

  TL;DR As we do for `absolute::LockTime` and for the same reasons; implement `ArbitraryOrd` for `relative::LockTime`.

  locktimes do not have a semantic ordering if they differ (blocks, time) so we do not derive `Ord` however it is useful for downstream to be able to order structs that contain lock times. This is exactly what the `ArbitraryOrd` trait is for.

  Fix: #2566

ACKs for top commit:
  sanket1729:
    ACK d91cdd2
  apoelstra:
    ACK d91cdd2

Tree-SHA512: 52ace9222e765dfa266d003b4aff3e93e35d1414c9fd579c4a4a36998d6d1b08bf6d4964a6f1c1d769068d65e47a882495daa4aacf254909a35dce8e01c99a9e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.32.0
Development

Successfully merging a pull request may close this issue.

Implement ArbitraryOrd for relative::LockTime tcharding/rust-bitcoin
3 participants
@apoelstra @tcharding @15IITian