Skip to content

refactor(chain): return named parameter structs from tBTC getters - #4188

Merged
piotr-roslaniec merged 1 commit into
mainfrom
refactor/named-chain-param-getters
Aug 7, 2026
Merged

refactor(chain): return named parameter structs from tBTC getters#4188
piotr-roslaniec merged 1 commit into
mainfrom
refactor/named-chain-param-getters

Conversation

@piotr-roslaniec

@piotr-roslaniec piotr-roslaniec commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator

Summary

The on-chain tBTC parameter getters returned wide positional tuples that
callers destructured with long blank-identifier lists. For example:

_, _, _, _, _, _, _, sweepTxMaxTotalFee, _, _, _, err := chain.GetMovingFundsParameters()

GetMovingFundsParameters returned 11 values plus err. Because many of the
values share a type, a positional slip in one of these lists compiles cleanly
and fails silently at runtime.

This replaces the wide tuples with named structs in pkg/tbtc, so callers
access fields by name:

movingFundsParameters, err := chain.GetMovingFundsParameters()
... movingFundsParameters.SweepTxMaxTotalFee ...

Changes

  • New pkg/tbtc/parameters.go defining MovingFundsParameters,
    WalletParameters, DepositParameters, RedemptionParameters.
  • GetMovingFundsParameters (on tbtc.BridgeChain, embedded by tbtcpg.Chain)
    and the three others (on tbtcpg.Chain) now return the corresponding struct.
  • All structs live in pkg/tbtc, which both the ethereum adapter and tbtcpg
    already import, so no new dependency edges are introduced.
  • Updated the ethereum adapter implementations, three inline-interface
    declarations in pkg/tbtc/moving_funds.go, both local test chains (which get
    simpler — their internal struct aliases are dropped), and all call sites.

Net: the diff removes more than it adds; the change is a simplification.

Notes for review

  • Each of the call-site conversions was mapped position-by-position against the
    original tuple order, and the tbtcpg tests discriminate a wrong-but-
    same-typed field pick (e.g. moved_funds_sweep sets TxMaxTotalFee to 0 but
    SweepTxMaxTotalFee to a non-zero value).
  • pkg/tbtc's test-only localChain.GetWalletParameters is left as the old
    wide-tuple panic("unsupported") stub. It satisfies no tbtc interface (only
    tbtcpg.Chain declares GetWalletParameters), so it is dead in the tbtc
    package and was left untouched to keep the change surgical.

Verification

  • go build ./... and go vet ./... clean.
  • pkg/tbtc, pkg/tbtcpg, and pkg/chain/ethereum test suites pass.

Summary by CodeRabbit

  • New Features

    • Added standardized parameter structures for deposits, redemptions, wallets, and moving funds.
    • Consolidated configuration retrieval into structured results, making fee limits, thresholds, timeouts, and balance settings easier to access.
  • Refactor

    • Updated related transaction, sweep, and wallet workflows to use the consolidated parameter structures without changing their existing behavior.

The on-chain parameter getters returned wide positional tuples (up to 11
values plus err) that callers destructured with long blank-identifier
lists, where a positional slip would compile silently. Replace them with
named structs (MovingFundsParameters, WalletParameters, DepositParameters,
RedemptionParameters) in pkg/tbtc so callers access fields by name.

GetMovingFundsParameters lives on tbtc.BridgeChain (embedded by
tbtcpg.Chain); the other three on tbtcpg.Chain. All structs live in
pkg/tbtc, which the ethereum adapter and tbtcpg already import.
@coderabbitai

coderabbitai Bot commented Jul 23, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 8d575896-3fdc-4c92-bff6-f556f7948505

📥 Commits

Reviewing files that changed from the base of the PR and between 5245f66 and c4cc0ad.

📒 Files selected for processing (11)
  • pkg/chain/ethereum/tbtc.go
  • pkg/tbtc/chain.go
  • pkg/tbtc/chain_test.go
  • pkg/tbtc/moving_funds.go
  • pkg/tbtc/parameters.go
  • pkg/tbtcpg/chain.go
  • pkg/tbtcpg/chain_test.go
  • pkg/tbtcpg/deposit_sweep.go
  • pkg/tbtcpg/moved_funds_sweep.go
  • pkg/tbtcpg/moving_funds.go
  • pkg/tbtcpg/redemptions.go

📝 Walkthrough

Walkthrough

TBTC parameter getters now return shared structs for deposit, redemption, wallet, and moving-funds settings. Interfaces, Ethereum and local implementations, validation logic, and fee-estimation consumers are updated to use the new struct fields.

Changes

TBTC parameter consolidation

Layer / File(s) Summary
Shared parameter contracts
pkg/tbtc/parameters.go, pkg/tbtc/chain.go, pkg/tbtc/moving_funds.go, pkg/tbtcpg/chain.go
Adds exported parameter structs and updates chain interfaces to return consolidated parameter values.
Chain implementations
pkg/chain/ethereum/tbtc.go, pkg/tbtc/chain_test.go, pkg/tbtcpg/chain_test.go
Maps bridge results and local test-chain state into the shared parameter structs.
Moving-funds validation
pkg/tbtc/moving_funds.go
Reads the moving-funds timeout from the returned parameter struct.
TBTC workflow consumers
pkg/tbtcpg/deposit_sweep.go, pkg/tbtcpg/moved_funds_sweep.go, pkg/tbtcpg/moving_funds.go, pkg/tbtcpg/redemptions.go
Updates fee estimation, wallet selection, and redemption handling to access parameter struct fields.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: tBTC getters now return named parameter structs instead of positional tuples.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/named-chain-param-getters

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@lrsaturnino lrsaturnino left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

No blocking issues found.

@piotr-roslaniec
piotr-roslaniec merged commit e48b1ed into main Aug 7, 2026
30 checks passed
@piotr-roslaniec
piotr-roslaniec deleted the refactor/named-chain-param-getters branch August 7, 2026 07:21
piotr-roslaniec added a commit that referenced this pull request Aug 18, 2026
…4191)

## TL;DR for reviewers

**This is a pure, AST-verified relocation — no declaration body
changed.** `pkg/chain/ethereum/tbtc.go` (~2400 lines, ~90 methods on
`TbtcChain`) is split into per-concern files within the same package.
Every top-level declaration was moved verbatim; a tool compared the AST
of all 96 declarations before and after the split and confirmed each is
byte-identical. Build, `go vet`, and the ethereum test suite all pass.
It should review in minutes, not hours: check the file assignments, not
the bodies.

## Why

The adapter had become a monolith spanning every tBTC concern
(sortition, DKG, inactivity, deposit, redemption, wallet, moving funds)
in one file, making it hard to navigate. Same-package split ⇒ no
visibility or interface-satisfaction changes.

## New file layout

| File | Concern |
|---|---|
| `tbtc.go` | `TbtcChain` struct, constructor, shared constants,
`TxProofDifficultyFactor` |
| `tbtc_sortition.go` | sortition pool, operator status, rewards, group
selection |
| `tbtc_dkg.go` | DKG lifecycle, result assembly and validation |
| `tbtc_inactivity.go` | inactivity claims |
| `tbtc_deposit.go` | deposit reveal, sweep proposals and proofs |
| `tbtc_redemption.go` | redemption requests, proposals and proofs |
| `tbtc_wallet.go` | wallet registration/state, heartbeat proposals |
| `tbtc_moving_funds.go` | moving funds + moved funds sweep |

## Notes / deviations

- **8 files, not the originally-scoped 4**
(deposit/redemption/wallet/moving-funds): DKG + sortition + inactivity
are ~40% of the file (~900 lines), so the 4-file version would have left
`tbtc.go` at ~1200 lines — still a monolith. Splitting those out too is
what actually de-monoliths the adapter.
- **Heartbeat** has a single method (`ValidateHeartbeatProposal`);
folded into `tbtc_wallet.go` (wallet liveness) rather than given its own
file.
- Imports were carried verbatim into each file and pruned per-file with
`goimports` (preserves the custom aliases
`ecdsaabi`/`tbtcabi`/`tbtccontract`/`ecdsacontract`).

## Base / stacking

Based on `refactor/named-chain-param-getters` (#4188), **not** `main`:
#4188 also edits `tbtc.go` (the `Get*Parameters` getters), so splitting
on top of it avoids a conflict. Once #4188 merges, this branch should be
rebased `--onto main`.
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.

2 participants