Skip to content

chore(chain): deprecate CanonicalIter, and ChainOracle trait#2140

Closed
oleonardolima wants to merge 1 commit intobitcoindevkit:release/chain-0.23.xfrom
oleonardolima:chore/deprecate-chain-oracle-and-canonical-iter
Closed

chore(chain): deprecate CanonicalIter, and ChainOracle trait#2140
oleonardolima wants to merge 1 commit intobitcoindevkit:release/chain-0.23.xfrom
oleonardolima:chore/deprecate-chain-oracle-and-canonical-iter

Conversation

@oleonardolima
Copy link
Collaborator

Description

As both CanonicalIter and ChainOracle trait are being removed/replaced in #2038, and they're public types, we should deprecated them in a 0.23.x release in case any user does use them directly.

Notes to the reviewers

It's simply deprecating CanonicalIter and ChainOracle trait.

Changelog notice

### Changed
- `CanonicalIter` and `ChainOracle` are planned to be removed in a future release, therefore have been deprecated.

Checklists

All Submissions:

@oleonardolima oleonardolima added this to the Chain 0.23.3 milestone Mar 9, 2026
@oleonardolima oleonardolima requested a review from evanlinjin March 9, 2026 18:57
@oleonardolima oleonardolima self-assigned this Mar 9, 2026
@evanlinjin
Copy link
Member

I'm wondering whether it really makes sense to deprecate things that we still rely on.

@notmandatory notmandatory moved this to Discussion in BDK Chain Mar 10, 2026
- add the deprecation warning for both `CanonicalIter` and `ChainOracle`
  trait, as they're planned to be removed in the future.
- add `#[allow(deprecated)] throughout the code to prevent related
  warnings.
@oleonardolima oleonardolima force-pushed the chore/deprecate-chain-oracle-and-canonical-iter branch from 532946a to 9286a90 Compare March 11, 2026 17:23
@oleonardolima oleonardolima added discussion There's still a discussion ongoing module-blockchain labels Mar 11, 2026
@oleonardolima
Copy link
Collaborator Author

I'm wondering whether it really makes sense to deprecate things that we still rely on.

I think it's the right approach, as CanonicalIter and ChainOracle are currently public types some user can be relying on them for some reason, and it's good to deprecated them as a notice that's planned for removal/migration.

Copy link
Member

@luisschwab luisschwab left a comment

Choose a reason for hiding this comment

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

ACK 9286a90

@evanlinjin
Copy link
Member

I don't think this is the right use of #[deprecated]. RFC 1270, which introduced #[deprecated], states its purpose is to let authors evolve APIs "while giving users enough time to react" — with the note field intended to describe "what to use instead." The design assumes a migration path exists at the time of deprecation. Here, the replacements (CanonicalTask, ChainQuery) don't exist in 0.23.x, so there's nothing for users to react to other than upgrading to a different major version.

A few concrete issues:

  • The crate itself needs #[allow(deprecated)] in 5 places to compile cleanly. When you have to suppress your own deprecation warnings throughout the same crate, it's a strong signal the deprecation is premature for this release line.
  • Every downstream consumer of 0.23.x will get warnings they cannot resolve without a major version upgrade. That's noise, not guidance.
  • The deprecation messages link to PR #2038 on master — that's not useful context for someone pinned to 0.23.x.

If the goal is to communicate that these APIs are going away, a doc comment note ("This will be replaced by X in version Y") or a mention in the release notes/changelog would achieve the same thing without polluting builds with unresolvable warnings.

@oleonardolima oleonardolima removed this from the Chain 0.23.3 milestone Mar 13, 2026
@oleonardolima
Copy link
Collaborator Author

As explained in the comment above, and discussed in the last dev call we don't need to deprecate these fields.

@oleonardolima oleonardolima deleted the chore/deprecate-chain-oracle-and-canonical-iter branch March 13, 2026 21:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

discussion There's still a discussion ongoing module-blockchain

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants