Skip to content

refactor: move tx_template from bdk_chain to bdk_testenv#1800

Closed
tvpeter wants to merge 1 commit intobitcoindevkit:masterfrom
tvpeter:test/move-txtemplate-to-testenv
Closed

refactor: move tx_template from bdk_chain to bdk_testenv#1800
tvpeter wants to merge 1 commit intobitcoindevkit:masterfrom
tvpeter:test/move-txtemplate-to-testenv

Conversation

@tvpeter
Copy link
Copy Markdown
Collaborator

@tvpeter tvpeter commented Jan 14, 2025

Description

This PR implements the following changes:

  • moves TxTemplate from crates/chain/tests/common/mod.rs to testenv.
  • refactors init_graph function and TxTemplate struct to use Cow
  • redoes the canonical benchmark tests introduced by Introduce O(n) canonicalization algorithm #1670 using TxTemplates

Closes #1936

Notes to the reviewers

Changelog notice

Checklists

All Submissions:

  • I've signed all my commits
  • I followed the contribution guidelines
  • I ran cargo fmt and cargo clippy before committing

@tvpeter tvpeter force-pushed the test/move-txtemplate-to-testenv branch 3 times, most recently from 8b81584 to 12181bb Compare January 21, 2025 10:30
@tvpeter tvpeter marked this pull request as ready for review January 21, 2025 10:34
@tvpeter tvpeter force-pushed the test/move-txtemplate-to-testenv branch from 12181bb to 13b87ac Compare January 23, 2025 11:53
@tvpeter tvpeter force-pushed the test/move-txtemplate-to-testenv branch from 13b87ac to dd47a8b Compare January 28, 2025 07:39
oleonardolima
oleonardolima reviewed Jan 28, 2025
View reviewed changes
Copy link
Copy Markdown
Collaborator

@oleonardolima oleonardolima left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for working on this one, so far it's looking good.

Please update the commit messages to follow conventional commits, i.e: `refactor: move `tx_template` from `bdk_chain` to `bdk_testenv`

Comment thread crates/testenv/Cargo.toml Outdated
Comment thread crates/testenv/src/tx_template.rs Outdated
@tvpeter tvpeter force-pushed the test/move-txtemplate-to-testenv branch from dd47a8b to 657ffd8 Compare January 29, 2025 08:57
@tvpeter tvpeter changed the title test(chain):move txtemplate to testenv refactor: move tx_template from bdk_chain to bdk_testenv Jan 29, 2025
@tvpeter tvpeter requested a review from oleonardolima January 29, 2025 09:02
@tvpeter tvpeter force-pushed the test/move-txtemplate-to-testenv branch from 657ffd8 to 1546ef6 Compare March 5, 2025 19:30
@oleonardolima
Copy link
Copy Markdown
Collaborator

oleonardolima commented Mar 7, 2025

You probably need a rebase to make sure CI is passing.

@tvpeter tvpeter force-pushed the test/move-txtemplate-to-testenv branch 2 times, most recently from 7d8ec4b to b42a074 Compare March 15, 2025 06:48
@tvpeter
refactor: move tx_template from chain to testenv
32cb79b 
- refactor tx_template struct and init_graph
to use Cow pointer
- move `tx_template` from `chain/tests/common` to
`testenv/src`
- update `chain/tests` files to use `tx_template`
from `testenv`
- update deps in `chain/cargo.toml` and
`testenv/cargo.toml`
- update imports in `testenv/src/lib.rs`
- refactor canonical benchmarks in `chain/benches`
 to use tx_template

[Ticket: 1754]
@tvpeter tvpeter force-pushed the test/move-txtemplate-to-testenv branch from b42a074 to 32cb79b Compare April 15, 2025 11:47
@notmandatory notmandatory moved this to In Progress in BDK Chain Apr 23, 2025
@va-an va-an mentioned this pull request Feb 22, 2026
Closed
6 tasks
@oleonardolima oleonardolima added this to the Chain 0.24.0 milestone Mar 19, 2026
@oleonardolima
Copy link
Copy Markdown
Collaborator

oleonardolima commented Mar 19, 2026

I've recently faced the issues of not having the TxTemplate under bdk_testenv, and the changes here would've helped.

@tvpeter this PR needs a big rebase.

@tvpeter
Copy link
Copy Markdown
Collaborator Author

tvpeter commented Apr 8, 2026

I will be splitting this PR into two because there are several changes across the affected crates. It seems more challenging to rebase and catch up. Once I link the two new PRs, I will close this one.

@evanlinjin @oleonardolima

@tvpeter
Copy link
Copy Markdown
Collaborator Author

tvpeter commented Apr 15, 2026

Closing this PR in favour of #2177

@tvpeter tvpeter closed this Apr 15, 2026
@github-project-automation github-project-automation Bot moved this from In Progress to Done in BDK Chain Apr 15, 2026
@oleonardolima
Copy link
Copy Markdown
Collaborator

oleonardolima commented Apr 28, 2026 •
edited
Loading

I will be splitting this PR into two because there are several changes across the affected crates. It seems more challenging to rebase and catch up. Once I link the two new PRs, I will close this one.

@evanlinjin @oleonardolima

Closing this PR in favour of #2177

@tvpeter Thanks! I'll take a look on that one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@oleonardolima oleonardolima Awaiting requested review from oleonardolima oleonardolima is a code owner

Assignees

@tvpeter tvpeter

Labels

tests

Projects

BDK Chain
Status: Done

Milestone

Chain 0.24.0

Development

Successfully merging this pull request may close these issues.

Move TxTemplate to bdk_testenv and make it better

3 participants

@tvpeter @oleonardolima @notmandatory