caravan-bitcoin / caravan

Caravan monorepo
https://caravanmultisig.com
MIT License
55 stars 48 forks source link

Improve Type System for `Caravan-Clients` Package #192

Open Legend101Zz opened 1 week ago

Legend101Zz commented 1 week ago

Summary

After digging through the codebase, I've noticed our type system in the caravan-clients package has several inconsistencies that make development harder than it needs to be. I'd like to propose a series of improvements to make our types more consistent, reduce any usage, and better align with Bitcoin RPC responses.

The Problems

Looking at the code, I've noticed a few issues:

Proposed Approach

I think we should tackle this in stages through several PRs:

Tasks

1. Centralize Type Definitions

2. Improve RPC Parameter Handling

3. Create Response Type Mapping

4. Make Core Functions Generic

5. Create Type Hierarchies for Complex Types

6. Implement Response Normalization Pattern

7. Replace any with Proper Type Guards

8. Documentation and Maintenance Strategy

Implementation Plan

I suggest we tackle this incrementally to avoid disrupting current development:

  1. First PR: Address items 1-2 (centralize types, fix RPC parameter handling).
  2. Second PR: Address items 3-4 (method mapping, generic functions).
  3. Third PR: Address items 5 (type hierarchies, replace any).
  4. Fourth PR: Address items 6 (normalisation functionality).
  5. Fifth PR: Address items 7 ( replace any).
  6. Sixth PR: Address item 8 (documentation and maintenance).

Why This Matters

These changes will:

I'm happy to tackle the first PR to get things started. What do you all think :)

Legend101Zz commented 1 week ago

Improve Type Consistency in caravan-clients

Tasks

🔗 Issue remains open until all tasks are completed.

Note: The above changes are temporary and should be adjusted based on needs and requirements.

cc: @bucko13 for visibility.

ice-009 commented 5 hours ago

I have completed the first two parts of the issue, mentioning the PR https://github.com/caravan-bitcoin/caravan/pull/200.

cc @Legend101Zz, @bucko13 for reviews