refactor(toolkit): add new BuilderContext trait (indexer support prep) by ozgb · Pull Request #1605 · midnightntwrk/midnight-node

ozgb · 2026-05-28T15:42:25Z

Overview

Encapsulate the methods toolkit transaction builders use behind a new BuilderContext<D> trait, so builders no longer depend on a concrete Arc<LedgerContext<D>> that has to replay every block locally.

The trait exposes only the queries builders actually need: latest_block_context, ledger_parameters, network_id, unshielded_utxos, zswap_state, contract_state, resolver/update_resolver, well_formed, and wallet access (with_wallet_from_seed, with_wallets_from_seeds). LedgerContext implements the trait, so runtime behaviour is unchanged. A stub IndexerContext proves the trait is implementable by a non-local backend.

A follow-up PR will add the real indexer-API implementation of BuilderContext (issue #1186), letting the toolkit drive transaction construction against a node via indexer queries instead of a fully replayed local ledger.

The batches command still uses the concrete LedgerContext backend - it needs a full ledger state to pre-generate and apply transactions to create future chain states.

🗹 TODO before merging

Ready

📌 Submission Checklist

All commits are signed off (git commit -s) for the DCO
Changes are backward-compatible (or flagged if breaking)
Pull request description explains why the change is needed
Self-reviewed the diff
I have included a change file, or skipped for this reason:
If the changes introduce a new feature, I have bumped the node minor version
Update documentation (if relevant)
Updated AGENTS.md if build commands, architecture, or workflows changed
No new todos introduced

🧪 Testing Evidence

Existing tests via CI should not show regression

🔱 Fork Strategy

Node Runtime Update
Node Client Update
Other: Toolkit-only refactor; no on-chain change.
N/A

Links

Issue [Toolkit] Add support for fetching data from the indexer #1186 — indexer-backed toolkit
Once [Toolkit] Add support for fetching data from the indexer #1186 is resolved, [toolkit] Toolkit consumes all available memory and crashes #1530 will be mitigated
Follow-up: IndexerContext implementation against the indexer GraphQL API

WIP abstraction: parameterise builders (BuildInput, BuildOutput, BuildTransient, BuildContractAction, BuildIntent, OfferInfo, StandardTrasactionInfo, SingleTxBuilder, FromContext) over a BuilderContext<D> trait instead of the concrete LedgerContext<D>. LedgerContext<D> implements BuilderContext<D>, with latest_block_context, ledger_parameters, and update_resolver still left as todo!() stubs. This is the first step toward letting toolkit drive an indexer-backed context (issue #1186) without downloading every block locally.

… guide Foundation for issue #1186 (indexer-backed toolkit). Finalizes the BuilderContext<D> trait surface (manual Pin<Box<Future>>, no async_trait): drops the dead tx_context and owned wallet_from_seed, and adds the indexer-validated query methods network_id, unshielded_utxos, zswap_state, contract_state, resolver, and a generic well_formed check. LedgerContext<D> implements every method (no stubs). utxo_output.rs (sync) and utxo_spend.rs (async) are migrated as reference templates for the remaining builders. docs/proposals/builder-context-migration.md documents the finalized trait, the indexer GraphQL API mapping, the per-file migration recipe, and the sandbox build/verify workaround. The crate does not yet compile: the remaining ~25 builder files still hold concrete Arc<LedgerContext> and are tracked in that guide.

Parameterise every tx_generator builder over a new BuilderContext<D> trait instead of a concrete LedgerContext<D>, exposing only the queries builders need (latest block context, ledger parameters, network id, unshielded UTXOs, zswap state, contract state, resolver, well-formedness, and wallet access). LedgerContext implements the trait so runtime behaviour is unchanged; an IndexerContext stub proves the trait is implementable by a non-local backend (issue #1186). Removes the now-redundant batches builder.

Replace manual `Pin<Box<dyn Future + Send + 'a>>` desugarings with the `#[async_trait]` macro on the traits that are heavily used via `dyn` (BuilderContext, BuildUtxoSpend, BuildIntent, BuildContractAction, Contract::deploy) and all of their impls. Cuts ~75 lines and removes a class of error-prone lifetime boilerplate; behavior is unchanged (ledger-helpers test suite still passes). Assisted-by: Claude:claude-4.7-opus

Resolve conflicts between BuilderContext trait abstraction and main: - Combine generic BuilderContext<D, C> signatures with main's API changes (Copy removal on WalletSeed; new dust spend/mark_spent tuple API; new --coin-selection strategy and input_utxos pinning in builders). - Migrate utxos_by_ids to BuilderContext, async. - Drop redundant batches builder (replaced by batch_single_tx). - Convert build_unshielded_intents overflow test to #[tokio::test]. Assisted-by: Claude:claude-4.7-opus Signed-off-by: Oscar Bailey <79094698+ozgb@users.noreply.github.com>

The BuilderContext trait refactor removed the batches builder because its inter-tx update_from_tx call is a write-side operation the trait deliberately omits. Re-add the builder pinned to the concrete LedgerContext<DefaultDB> so the existing CLI surface keeps working, update the trait-object signatures to the new <DefaultDB, C> shape, make initial_unshielded_intents async (utxos_to_cover_value is now async), and thread .clone() through call sites since WalletSeed is no longer Copy. Adds --coin-selection to BatchesArgs to match the other builders. Updates the change file and migration doc to reflect that batches was kept rather than removed. Assisted-by: Claude:claude-4.7-opus Signed-off-by: Oscar Bailey <79094698+ozgb@users.noreply.github.com>

Signed-off-by: Oscar Bailey <79094698+ozgb@users.noreply.github.com>

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c0901a8ba7

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Signed-off-by: Oscar Bailey <79094698+ozgb@users.noreply.github.com>

…github.com> I, Oscar Bailey <79094698+ozgb@users.noreply.github.com>, hereby add my Signed-off-by to this commit: eb16d26 I, Oscar Bailey <79094698+ozgb@users.noreply.github.com>, hereby add my Signed-off-by to this commit: 6990ef2 I, Oscar Bailey <79094698+ozgb@users.noreply.github.com>, hereby add my Signed-off-by to this commit: e935344 I, Oscar Bailey <79094698+ozgb@users.noreply.github.com>, hereby add my Signed-off-by to this commit: b97f86d Signed-off-by: Oscar Bailey <79094698+ozgb@users.noreply.github.com>

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 19fe8a5101

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Signed-off-by: Oscar Bailey <79094698+ozgb@users.noreply.github.com>

Klapeyron · 2026-06-02T11:44:07Z

-	pub intents: HashMap<SegmentId, Box<dyn BuildIntent<D>>>,
-	pub guaranteed_offer: Option<OfferInfo<D>>,
-	pub fallible_offers: HashMap<u16, OfferInfo<D>>,
+pub struct StandardTrasactionInfo<D: DB + Clone, C: BuilderContext<D>> {


Suggested change

pub struct StandardTrasactionInfo<D: DB + Clone, C: BuilderContext<D>> {

pub struct StandardTransactionInfo<D: DB + Clone, C: BuilderContext<D>> {

Will apply in a follow-up

Fixed here: #1627

Klapeyron · 2026-06-02T11:57:56Z

+be satisfied by a non-local backend, preparing the toolkit to talk to a node via
+indexer queries (issue #1186) without touching the builders again.
+
+No user-visible behaviour change. The `batches` builder retains its previous CLI


will it be worth to introduce additional trait that will contain the mutable operations only? Looks like we need functions like update_from_tx or block_context

The indexer backend is unlikely to support mutable operations - unless we expect that to change, I don't see benefit in creating the additional trait

Klapeyron · 2026-06-02T12:06:51Z

+	async fn resolver(&self) -> &'static Resolver;
+
+	/// Replace the resolver used for proving.
+	async fn update_resolver(&self, resolver: &'static Resolver);


some of the functions are shared among LedgerContext and BuilderContext like this one or with_wallet_from_seed. latest_block_context for instance is sometimes used via trait as async function or directly without async, can we try to unify that?

I think this is OK and expected - some of the trait API will have a 1:1 mapping with the backend

EDIT: Can you point to the specific usage?

Klapeyron · 2026-06-02T12:12:43Z

+		self.with_ledger_state(|ledger_state| {
+			self.with_wallet_from_seed(seed, |wallet| {


here we are making two mutex locks (wallet and ledger_state), which may cause deadlock if somewhere else we make it in opposite direction. Maybe we can try to extract addresses first before second lock is aquired?

Good catch, thanks - will fix this in a follow-up

I wasn't able to neatly remove this double-locking - I've document the correct order, and verified that all current usage is the same order: #1626

ozgb added 8 commits May 26, 2026 13:38

refactor: more explicit variable naming

64eb921

Signed-off-by: Oscar Bailey <79094698+ozgb@users.noreply.github.com>

docs: remove outdated implementation plan

c0901a8

Signed-off-by: Oscar Bailey <79094698+ozgb@users.noreply.github.com>

ozgb requested a review from a team as a code owner May 28, 2026 15:42

ozgb added the ai-assisted Created or modified with AI assistance label May 28, 2026

chatgpt-codex-connector Bot reviewed May 28, 2026

View reviewed changes

Comment thread util/toolkit/src/tx_generator/builder/builders/common/batches.rs Outdated

ozgb and others added 4 commits May 28, 2026 16:47

docs: update change file and move to toolkit changes folder

0f45c17

Signed-off-by: Oscar Bailey <79094698+ozgb@users.noreply.github.com>

fix: re-apply missing overflow check

ea39770

Signed-off-by: Oscar Bailey <79094698+ozgb@users.noreply.github.com>

Merge branch 'main' into ozgb-builder-context-trait

5b98d49

chatgpt-codex-connector Bot reviewed May 29, 2026

View reviewed changes

Comment thread ledger/helpers/src/versions/common/context/builder_context.rs

chore: add missing license header

978cc7f

Signed-off-by: Oscar Bailey <79094698+ozgb@users.noreply.github.com>

gilescope approved these changes Jun 2, 2026

View reviewed changes

gilescope added this pull request to the merge queue Jun 2, 2026

Klapeyron reviewed Jun 2, 2026

View reviewed changes

Merged via the queue into main with commit 4fd691e Jun 2, 2026
57 of 58 checks passed

gilescope deleted the ozgb-builder-context-trait branch June 2, 2026 12:43

This was referenced Jun 2, 2026

docs: document correct order of locking for LedgerContext #1626

Open

refactor: fix typo StandardTrasactionInfo -> StandardTransactionInfo #1627

Open

gilescope self-assigned this Jun 4, 2026

DarrenOliveiro-Priestnall added the component:node Node component label Jun 5, 2026

	pub struct StandardTrasactionInfo<D: DB + Clone, C: BuilderContext<D>> {
	pub struct StandardTransactionInfo<D: DB + Clone, C: BuilderContext<D>> {

		self.with_ledger_state(\|ledger_state\| {
		self.with_wallet_from_seed(seed, \|wallet\| {

Uh oh!

Conversation

ozgb commented May 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overview

🗹 TODO before merging

📌 Submission Checklist

🧪 Testing Evidence

🔱 Fork Strategy

Links

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ozgb Jun 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ozgb commented May 28, 2026 •

edited

Loading

ozgb Jun 2, 2026 •

edited

Loading