Skip to content

feat: add btcAddress action - #911

Open
shuaixr wants to merge 2 commits into
open-circle:mainfrom
shuaixr:feat-add-btc-address-action
Open

feat: add btcAddress action#911
shuaixr wants to merge 2 commits into
open-circle:mainfrom
shuaixr:feat-add-btc-address-action

Conversation

@shuaixr

@shuaixr shuaixr commented Nov 4, 2024

Copy link
Copy Markdown

Perform checksum validation for a btc address

Summary by CodeRabbit

  • New Features
    • Added Bitcoin address validation actions for legacy Base58Check and native SegWit Bech32/Bech32m, plus Base58Check-only and Bech32/Bech32m-only variants.
    • Added support for custom validation messages with strong TypeScript inference.
    • Exposed the new actions, issue/action contracts, and related validation utilities via the public API.
  • Documentation
    • Added API reference pages for the new actions and their generic types, including usage examples and validation scope/limitations.
  • Tests
    • Added runtime and compile-time tests covering valid/invalid addresses, issue generation, and message/type inference.

@fabian-hiller

fabian-hiller commented Nov 4, 2024

Copy link
Copy Markdown
Member

Thanks for your PR! Is there any way we can simplify the bitcoin address check with a simple regular expression and checksum function? Just a random question. I haven't done any research yet.

@fabian-hiller fabian-hiller self-assigned this Nov 4, 2024
@fabian-hiller fabian-hiller added enhancement New feature or request question Further information is requested labels Nov 4, 2024
@shuaixr

shuaixr commented Nov 4, 2024

Copy link
Copy Markdown
Author

Is there any way we can simplify the bitcoin address check with a simple regular expression and checksum function?

sure, I'll add a regular expression, first using regex then the checksum.

@fabian-hiller

fabian-hiller commented Nov 5, 2024

Copy link
Copy Markdown
Member

sure, I'll add a regular expression, first using regex then the checksum.

Is it possible to simplify the implementation in this way and reduce the bundle size without losing relevant functionality? The current implementation contains a lot of functions and seems quite complicated.

@shuaixr

shuaixr commented Nov 5, 2024

Copy link
Copy Markdown
Author

Is it possible to simplify the implementation in this way and reduce the bundle size without losing relevant functionality? The current implementation contains a lot of functions and seems quite complicated.

I can use crypto.subtle.digest instead of a library/src/actions/btcAddress/sha256-uint8array.ts file. It's supported in Node 15+, Bun, Deno, and browsers. i think all other functions are still necessary

update:
I noticed crypto.subtle.digest is async, so I'm not sure if the whole action needs to be async.

I've already simplified the code as much as possible, and the Bech32 decoding has removed any parts unrelated to validation: https://github.com/bitcoin/bips/blob/master/bip-0173.mediawiki#reference-implementations

@fabian-hiller

Copy link
Copy Markdown
Member

Thank you for your research. I want to let you know that I am focusing on our v1 release first before reviewing this PR.

@fabian-hiller
fabian-hiller force-pushed the main branch 2 times, most recently from f41426c to d713dfe Compare January 4, 2025 00:55
@fabian-hiller fabian-hiller added priority This has priority and removed question Further information is requested labels Jan 23, 2025
@fabian-hiller fabian-hiller removed the priority This has priority label Mar 16, 2025
@fabian-hiller fabian-hiller added this to the v1.2 milestone Apr 20, 2025
@dosubot dosubot Bot added the size:XL This PR changes 500-999 lines, ignoring generated files. label Jul 18, 2026
@coderabbitai

coderabbitai Bot commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 1cc817eb-0f00-459d-a8d8-84e8ad42dd8f

📥 Commits

Reviewing files that changed from the base of the PR and between 389feea and 7e7369f.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (47)
  • library/package.json
  • library/src/actions/btcAddress/btcAddress.test-d.ts
  • library/src/actions/btcAddress/btcAddress.test.ts
  • library/src/actions/btcAddress/btcAddress.ts
  • library/src/actions/btcAddress/index.ts
  • library/src/actions/btcAddressBase58/btcAddressBase58.test-d.ts
  • library/src/actions/btcAddressBase58/btcAddressBase58.test.ts
  • library/src/actions/btcAddressBase58/btcAddressBase58.ts
  • library/src/actions/btcAddressBase58/index.ts
  • library/src/actions/btcAddressBech32/btcAddressBech32.test-d.ts
  • library/src/actions/btcAddressBech32/btcAddressBech32.test.ts
  • library/src/actions/btcAddressBech32/btcAddressBech32.ts
  • library/src/actions/btcAddressBech32/index.ts
  • library/src/actions/index.ts
  • library/src/regex.ts
  • library/src/utils/_isBase58Check/_isBase58Check.test.ts
  • library/src/utils/_isBase58Check/_isBase58Check.ts
  • library/src/utils/_isBase58Check/index.ts
  • library/src/utils/_isBech32/_isBech32.test.ts
  • library/src/utils/_isBech32/_isBech32.ts
  • library/src/utils/_isBech32/index.ts
  • library/src/utils/_isBtcAddressBase58/_isBtcAddressBase58.test.ts
  • library/src/utils/_isBtcAddressBase58/_isBtcAddressBase58.ts
  • library/src/utils/_isBtcAddressBase58/index.ts
  • library/src/utils/_isBtcAddressBech32/_isBtcAddressBech32.test.ts
  • library/src/utils/_isBtcAddressBech32/_isBtcAddressBech32.ts
  • library/src/utils/_isBtcAddressBech32/index.ts
  • library/src/utils/index.ts
  • website/src/routes/api/(actions)/btcAddress/index.mdx
  • website/src/routes/api/(actions)/btcAddress/properties.ts
  • website/src/routes/api/(actions)/btcAddressBase58/index.mdx
  • website/src/routes/api/(actions)/btcAddressBase58/properties.ts
  • website/src/routes/api/(actions)/btcAddressBech32/index.mdx
  • website/src/routes/api/(actions)/btcAddressBech32/properties.ts
  • website/src/routes/api/(types)/BtcAddressAction/index.mdx
  • website/src/routes/api/(types)/BtcAddressAction/properties.ts
  • website/src/routes/api/(types)/BtcAddressBase58Action/index.mdx
  • website/src/routes/api/(types)/BtcAddressBase58Action/properties.ts
  • website/src/routes/api/(types)/BtcAddressBase58Issue/index.mdx
  • website/src/routes/api/(types)/BtcAddressBase58Issue/properties.ts
  • website/src/routes/api/(types)/BtcAddressBech32Action/index.mdx
  • website/src/routes/api/(types)/BtcAddressBech32Action/properties.ts
  • website/src/routes/api/(types)/BtcAddressBech32Issue/index.mdx
  • website/src/routes/api/(types)/BtcAddressBech32Issue/properties.ts
  • website/src/routes/api/(types)/BtcAddressIssue/index.mdx
  • website/src/routes/api/(types)/BtcAddressIssue/properties.ts
  • website/src/routes/api/menu.md
🚧 Files skipped from review as they are similar to previous changes (34)
  • library/src/actions/btcAddressBech32/index.ts
  • library/src/actions/btcAddress/index.ts
  • library/src/utils/_isBase58Check/index.ts
  • website/src/routes/api/(types)/BtcAddressBase58Action/index.mdx
  • library/src/actions/btcAddressBase58/index.ts
  • website/src/routes/api/(types)/BtcAddressBech32Action/index.mdx
  • website/src/routes/api/(actions)/btcAddressBech32/properties.ts
  • website/src/routes/api/(types)/BtcAddressBase58Issue/index.mdx
  • library/src/actions/index.ts
  • website/src/routes/api/(types)/BtcAddressIssue/index.mdx
  • website/src/routes/api/(types)/BtcAddressBech32Issue/properties.ts
  • library/src/actions/btcAddressBase58/btcAddressBase58.test-d.ts
  • website/src/routes/api/(actions)/btcAddressBase58/properties.ts
  • website/src/routes/api/(types)/BtcAddressBase58Action/properties.ts
  • library/src/utils/_isBech32/_isBech32.test.ts
  • website/src/routes/api/(types)/BtcAddressIssue/properties.ts
  • website/src/routes/api/(actions)/btcAddress/properties.ts
  • library/src/utils/_isBase58Check/_isBase58Check.test.ts
  • website/src/routes/api/(types)/BtcAddressAction/index.mdx
  • library/src/actions/btcAddressBase58/btcAddressBase58.test.ts
  • library/src/utils/_isBech32/index.ts
  • library/src/actions/btcAddressBech32/btcAddressBech32.test.ts
  • website/src/routes/api/(types)/BtcAddressBech32Action/properties.ts
  • library/src/utils/_isBech32/_isBech32.ts
  • website/src/routes/api/(types)/BtcAddressBech32Issue/index.mdx
  • website/src/routes/api/(types)/BtcAddressBase58Issue/properties.ts
  • website/src/routes/api/(types)/BtcAddressAction/properties.ts
  • website/src/routes/api/menu.md
  • library/package.json
  • library/src/actions/btcAddress/btcAddress.test.ts
  • library/src/actions/btcAddressBech32/btcAddressBech32.test-d.ts
  • library/src/utils/_isBase58Check/_isBase58Check.ts
  • website/src/routes/api/(actions)/btcAddress/index.mdx
  • library/src/regex.ts

Walkthrough

Adds Base58Check and Bech32/Bech32m checksum utilities, typed Bitcoin address validation actions, public barrel exports, runtime and declaration tests, and a dependency on @noble/hashes. The combined action supports legacy Base58Check and native SegWit addresses, while separate actions validate each format. New API documentation covers the actions, issue and action interfaces, property metadata, examples, related APIs, and navigation entries.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title is concise and accurately summarizes the main change: adding the btcAddress action.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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.

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
library/src/actions/btcAddress/sha256-uint8array.ts (1)

1-398: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Move this internal SHA-256 helper to utils/.

This file is a utility, not an action. Relocate it under library/src/utils/ with an underscore-prefixed filename and update its import.

As per coding guidelines, internal utilities belong in utils/ with an underscore prefix, while action files implement action objects with a '~run' method.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@library/src/actions/btcAddress/sha256-uint8array.ts` around lines 1 - 398,
Move the SHA-256 implementation containing createHash and Hash from the actions
directory into library/src/utils/ under an underscore-prefixed filename,
preserving its exports and behavior. Update every import or reference to
createHash/Hash to use the relocated utility, and remove the original
action-path file.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@library/src/actions/btcAddress/btcAddress.test.ts`:
- Line 19: Rename the validation action method from '~validate' to '~run'
throughout the btcAddress action contract. In
library/src/actions/btcAddress/btcAddress.test.ts lines 19 and 64, update the
baseAction expectation and invocation respectively; also update the
corresponding method key in btcAddress.ts so the implementation matches the
latest API.

In `@library/src/actions/btcAddress/btcAddress.ts`:
- Around line 227-235: Update the Bech32 selection in requirement(address) to
derive the encoding from the normalized witness version rather than the
case-sensitive “bc1p” prefix. Accept uppercase Taproot addresses by normalizing
the address for prefix/version inspection, and use Bech32m for every nonzero
witness version while retaining Bech32 for version zero.
- Around line 194-199: Add the `// `@__NO_SIDE_EFFECTS__`` annotation immediately
before the pure `btcAddress` factory function declaration. Keep the function
signature and implementation unchanged.
- Around line 264-269: Replace the returned action object's '~validate' method
with the required '~run' method, preserving the existing dataset typed-check and
btc address requirement validation logic so the pipeline can execute the action.

In `@library/src/regex.ts`:
- Around line 18-19: Update BTC_ADDRESS_REGEX to accept legacy testnet Base58
prefixes m, n, and 2 in addition to the existing mainnet prefixes, while
preserving the current Bech32 patterns and length constraints so btcAddress.ts
can validate the supported version bytes.

---

Outside diff comments:
In `@library/src/actions/btcAddress/sha256-uint8array.ts`:
- Around line 1-398: Move the SHA-256 implementation containing createHash and
Hash from the actions directory into library/src/utils/ under an
underscore-prefixed filename, preserving its exports and behavior. Update every
import or reference to createHash/Hash to use the relocated utility, and remove
the original action-path file.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 287375f7-799e-4178-b10f-6c101f70f50c

📥 Commits

Reviewing files that changed from the base of the PR and between 32247b3 and c21f53d.

📒 Files selected for processing (7)
  • library/src/actions/btcAddress/btcAddress.test-d.ts
  • library/src/actions/btcAddress/btcAddress.test.ts
  • library/src/actions/btcAddress/btcAddress.ts
  • library/src/actions/btcAddress/index.ts
  • library/src/actions/btcAddress/sha256-uint8array.ts
  • library/src/actions/index.ts
  • library/src/regex.ts

expects: null,
async: false,
requirement: expect.any(Function),
'~validate': expect.any(Function),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Update action method from '~validate' to '~run'.

The codebase and coding guidelines require validation actions to use the '~run' method. These tests still reference the older '~validate' method name, which will cause test failures when interfacing with the latest internal types and helpers.

  • library/src/actions/btcAddress/btcAddress.test.ts#L19-L19: change '~validate': expect.any(Function), to '~run': expect.any(Function), in the baseAction definition.
  • library/src/actions/btcAddress/btcAddress.test.ts#L64-L64: change the action['~validate'](...) invocation to action['~run'](...).

Note: The actual implementation in btcAddress.ts also appears to use '~validate' and will need to be updated to '~run' to match the API if not already done.

📍 Affects 1 file
  • library/src/actions/btcAddress/btcAddress.test.ts#L19-L19 (this comment)
  • library/src/actions/btcAddress/btcAddress.test.ts#L64-L64
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@library/src/actions/btcAddress/btcAddress.test.ts` at line 19, Rename the
validation action method from '~validate' to '~run' throughout the btcAddress
action contract. In library/src/actions/btcAddress/btcAddress.test.ts lines 19
and 64, update the baseAction expectation and invocation respectively; also
update the corresponding method key in btcAddress.ts so the implementation
matches the latest API.

Source: Coding guidelines

Comment thread library/src/actions/btcAddress/btcAddress.ts Outdated
Comment thread library/src/actions/btcAddress/btcAddress.ts Outdated
Comment thread library/src/actions/btcAddress/btcAddress.ts Outdated
Comment thread library/src/regex.ts Outdated
@yslpn

yslpn commented Jul 18, 2026

Copy link
Copy Markdown
Member

Hi @shuaixr Thanks for your input. I have a few thoughts.

We have a use case for splitting into three actions: ip/ipv4/ipv6. Do you think we could do the same here? For example, I see that we could split into btcAddressBase58, btcAddressBech32, and then combine btcAddress. This is especially important, as btcAddressBech32 users won't need the heavy sha256 dependency.

What about other cryptocurrencies in the future? Similar to credit card validation, move _base58Check and _bech32 out and then create thin wrappers that reuse the logic. Let's move the logic directly to library/src/utils/.

Another question is whether we need the first dependency in the project or not. sha256-uint8array is a library that was last updated six years ago. Moreover, it hasn't undergone a security audit. I believe the most modern and secure alternative is https://github.com/paulmillr/noble-hashes

It would also be great if you could write some website documentation for your check, so users know exactly what it checks and what the limitations are.

@shuaixr

shuaixr commented Jul 19, 2026

Copy link
Copy Markdown
Author

Hi @yslpn , sure. It’s been quite a while, so I’ll take another look and address this.

@shuaixr
shuaixr force-pushed the feat-add-btc-address-action branch from c21f53d to 389feea Compare July 19, 2026 08:49
@dosubot dosubot Bot added size:XXL This PR changes 1000+ lines, ignoring generated files. and removed size:XL This PR changes 500-999 lines, ignoring generated files. labels Jul 19, 2026
@shuaixr

shuaixr commented Jul 19, 2026

Copy link
Copy Markdown
Author

I introduced @noble/hashes as Valibot first runtime dependency for the Base58Check checksum

Browser Bundle size check:

Action Raw Gzip
btcAddressBech32 3.88 KB 1.57 KB
btcAddressBase58 9.16 KB 3.82 KB
btcAddress 11.53 KB 4.59 KB

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@library/src/actions/btcAddress/btcAddress.ts`:
- Around line 13-124: Remove the duplicated constants and helpers from
library/src/actions/btcAddress/btcAddress.ts (13-124), instantiate
btcAddressBase58() and btcAddressBech32() once at module scope, and compose
btcAddress.requirement from their exported requirement functions. Keep
library/src/actions/btcAddressBech32/btcAddressBech32.ts (12-115) as the
canonical Bech32 implementation,
library/src/actions/btcAddressBase58/btcAddressBase58.ts (12) as the canonical
Base58 version source, and update library/src/utils/_isBech32/_isBech32.ts (9)
to export BECH32_CHARSET or an equivalent witness-version helper; these sibling
sites should provide reusable exports rather than copied logic.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 316d922a-925a-4407-addf-1e0c152368d8

📥 Commits

Reviewing files that changed from the base of the PR and between c21f53d and 389feea.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (41)
  • library/package.json
  • library/src/actions/btcAddress/btcAddress.test-d.ts
  • library/src/actions/btcAddress/btcAddress.test.ts
  • library/src/actions/btcAddress/btcAddress.ts
  • library/src/actions/btcAddress/index.ts
  • library/src/actions/btcAddressBase58/btcAddressBase58.test-d.ts
  • library/src/actions/btcAddressBase58/btcAddressBase58.test.ts
  • library/src/actions/btcAddressBase58/btcAddressBase58.ts
  • library/src/actions/btcAddressBase58/index.ts
  • library/src/actions/btcAddressBech32/btcAddressBech32.test-d.ts
  • library/src/actions/btcAddressBech32/btcAddressBech32.test.ts
  • library/src/actions/btcAddressBech32/btcAddressBech32.ts
  • library/src/actions/btcAddressBech32/index.ts
  • library/src/actions/index.ts
  • library/src/regex.ts
  • library/src/utils/_isBase58Check/_isBase58Check.test.ts
  • library/src/utils/_isBase58Check/_isBase58Check.ts
  • library/src/utils/_isBase58Check/index.ts
  • library/src/utils/_isBech32/_isBech32.test.ts
  • library/src/utils/_isBech32/_isBech32.ts
  • library/src/utils/_isBech32/index.ts
  • library/src/utils/index.ts
  • website/src/routes/api/(actions)/btcAddress/index.mdx
  • website/src/routes/api/(actions)/btcAddress/properties.ts
  • website/src/routes/api/(actions)/btcAddressBase58/index.mdx
  • website/src/routes/api/(actions)/btcAddressBase58/properties.ts
  • website/src/routes/api/(actions)/btcAddressBech32/index.mdx
  • website/src/routes/api/(actions)/btcAddressBech32/properties.ts
  • website/src/routes/api/(types)/BtcAddressAction/index.mdx
  • website/src/routes/api/(types)/BtcAddressAction/properties.ts
  • website/src/routes/api/(types)/BtcAddressBase58Action/index.mdx
  • website/src/routes/api/(types)/BtcAddressBase58Action/properties.ts
  • website/src/routes/api/(types)/BtcAddressBase58Issue/index.mdx
  • website/src/routes/api/(types)/BtcAddressBase58Issue/properties.ts
  • website/src/routes/api/(types)/BtcAddressBech32Action/index.mdx
  • website/src/routes/api/(types)/BtcAddressBech32Action/properties.ts
  • website/src/routes/api/(types)/BtcAddressBech32Issue/index.mdx
  • website/src/routes/api/(types)/BtcAddressBech32Issue/properties.ts
  • website/src/routes/api/(types)/BtcAddressIssue/index.mdx
  • website/src/routes/api/(types)/BtcAddressIssue/properties.ts
  • website/src/routes/api/menu.md
🚧 Files skipped from review as they are similar to previous changes (4)
  • library/src/actions/index.ts
  • library/src/actions/btcAddress/index.ts
  • library/src/actions/btcAddress/btcAddress.test-d.ts
  • library/src/actions/btcAddress/btcAddress.test.ts

Comment on lines +13 to +124
/**
* Bitcoin Base58Check version bytes.
*/
const BITCOIN_BASE58_VERSIONS = [0x00, 0x05, 0x6f, 0xc4] as const;

/**
* Bitcoin Bech32 human readable parts.
*/
const BITCOIN_BECH32_HRPS = ['bc', 'tb'] as const;

/**
* Bech32 charset.
*/
const BECH32_CHARSET = 'qpzry9x8gf2tvdw0s3jn54khce6mua7l';

/**
* Converts bits.
*
* @param data The data values.
* @param from The source bit group size.
* @param to The target bit group size.
*
* @returns The converted bits.
*/
// @__NO_SIDE_EFFECTS__
function convertBits(
data: readonly number[],
from: number,
to: number
): number[] | null {
let accumulator = 0;
let bits = 0;
const result: number[] = [];
const maxValue = (1 << to) - 1;
const maxAccumulator = (1 << (from + to - 1)) - 1;

for (const value of data) {
accumulator = ((accumulator << from) | value) & maxAccumulator;
bits += from;

while (bits >= to) {
bits -= to;
result.push((accumulator >> bits) & maxValue);
}
}

if (bits >= from || ((accumulator << (to - bits)) & maxValue) !== 0) {
return null;
}

return result;
}

/**
* Gets the Bitcoin witness version.
*
* @param input The Bitcoin Bech32 address.
*
* @returns The Bitcoin witness version.
*/
// @__NO_SIDE_EFFECTS__
function getWitnessVersion(input: string): number {
const normalized = input.toLowerCase();
const separator = normalized.lastIndexOf('1');
return BECH32_CHARSET.indexOf(normalized[separator + 1]);
}

/**
* Checks if Bech32 data is a valid Bitcoin witness payload.
*
* @param data The Bech32 data.
*
* @returns Whether the Bech32 data is a valid Bitcoin witness payload.
*/
// @__NO_SIDE_EFFECTS__
function isBitcoinWitnessPayload(data: readonly number[]): boolean {
const program = convertBits(data.slice(1), 5, 8);

return !!(
program &&
program.length >= 2 &&
program.length <= 40 &&
(data[0] !== 0 || program.length === 20 || program.length === 32)
);
}

/**
* Checks if a string is a Bitcoin Bech32 or Bech32m address.
*
* @param input The string input.
*
* @returns Whether the string is a Bitcoin Bech32 or Bech32m address.
*/
// @__NO_SIDE_EFFECTS__
function isBtcAddressBech32(input: string): boolean {
if (!BTC_ADDRESS_BECH32_REGEX.test(input)) {
return false;
}

const witnessVersion = getWitnessVersion(input);

if (witnessVersion < 0 || witnessVersion > 16) {
return false;
}

return _isBech32(
input,
BITCOIN_BECH32_HRPS,
witnessVersion === 0 ? 'bech32' : 'bech32m',
isBitcoinWitnessPayload
);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Deduplicate the Bech32/Base58 helper logic that's now copy-pasted across three action files.

btcAddress.ts re-implements convertBits, getWitnessVersion, isBitcoinWitnessPayload, isBtcAddressBech32, BECH32_CHARSET, and BITCOIN_BECH32_HRPS byte-for-byte from btcAddressBech32.ts, and duplicates BITCOIN_BASE58_VERSIONS from btcAddressBase58.ts. This is the exact duplication the PR discussion (yslpn) asked to be resolved by moving reusable _base58Check/_bech32 logic into library/src/utils/ — instead the combined action grew its own copy rather than delegating to the sibling actions it wraps.

  • library/src/actions/btcAddress/btcAddress.ts#L13-L124: drop all of these locally-duplicated constants/helpers; instantiate btcAddressBase58() and btcAddressBech32() once at module scope and reuse their exported .requirement functions in btcAddress's own requirement.
  • library/src/actions/btcAddressBech32/btcAddressBech32.ts#L12-L115: keep as the canonical source of Bech32 witness-version logic that btcAddress.ts should import/reuse instead of copying.
  • library/src/actions/btcAddressBase58/btcAddressBase58.ts#L12-L12: keep as the canonical source of BITCOIN_BASE58_VERSIONS that btcAddress.ts should import/reuse instead of copying.
  • library/src/utils/_isBech32/_isBech32.ts#L9-L9: export BECH32_CHARSET (or a small witness-version helper) from this module so callers don't need to redefine the charset locally.
♻️ Sketch of the reuse-based fix for btcAddress.ts
-import {
-  BTC_ADDRESS_BASE58_REGEX,
-  BTC_ADDRESS_BECH32_REGEX,
-  BTC_ADDRESS_REGEX,
-} from '../../regex.ts';
+import { BTC_ADDRESS_REGEX } from '../../regex.ts';
 import type {
   BaseIssue,
   BaseValidation,
   ErrorMessage,
 } from '../../types/index.ts';
-import { _addIssue, _isBase58Check, _isBech32 } from '../../utils/index.ts';
-
-const BITCOIN_BASE58_VERSIONS = [0x00, 0x05, 0x6f, 0xc4] as const;
-const BITCOIN_BECH32_HRPS = ['bc', 'tb'] as const;
-const BECH32_CHARSET = 'qpzry9x8gf2tvdw0s3jn54khce6mua7l';
-
-function convertBits(...) { ... }
-function getWitnessVersion(...) { ... }
-function isBitcoinWitnessPayload(...) { ... }
-function isBtcAddressBech32(...) { ... }
+import { _addIssue } from '../../utils/index.ts';
+import { btcAddressBase58 } from '../btcAddressBase58/btcAddressBase58.ts';
+import { btcAddressBech32 } from '../btcAddressBech32/btcAddressBech32.ts';
+
+const base58Requirement = btcAddressBase58().requirement;
+const bech32Requirement = btcAddressBech32().requirement;
     requirement(input) {
       return (
         BTC_ADDRESS_REGEX.test(input) &&
-        ((BTC_ADDRESS_BASE58_REGEX.test(input) &&
-          _isBase58Check(input, BITCOIN_BASE58_VERSIONS)) ||
-          isBtcAddressBech32(input))
+        (base58Requirement(input) || bech32Requirement(input))
       );
     },
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/**
* Bitcoin Base58Check version bytes.
*/
const BITCOIN_BASE58_VERSIONS = [0x00, 0x05, 0x6f, 0xc4] as const;
/**
* Bitcoin Bech32 human readable parts.
*/
const BITCOIN_BECH32_HRPS = ['bc', 'tb'] as const;
/**
* Bech32 charset.
*/
const BECH32_CHARSET = 'qpzry9x8gf2tvdw0s3jn54khce6mua7l';
/**
* Converts bits.
*
* @param data The data values.
* @param from The source bit group size.
* @param to The target bit group size.
*
* @returns The converted bits.
*/
// @__NO_SIDE_EFFECTS__
function convertBits(
data: readonly number[],
from: number,
to: number
): number[] | null {
let accumulator = 0;
let bits = 0;
const result: number[] = [];
const maxValue = (1 << to) - 1;
const maxAccumulator = (1 << (from + to - 1)) - 1;
for (const value of data) {
accumulator = ((accumulator << from) | value) & maxAccumulator;
bits += from;
while (bits >= to) {
bits -= to;
result.push((accumulator >> bits) & maxValue);
}
}
if (bits >= from || ((accumulator << (to - bits)) & maxValue) !== 0) {
return null;
}
return result;
}
/**
* Gets the Bitcoin witness version.
*
* @param input The Bitcoin Bech32 address.
*
* @returns The Bitcoin witness version.
*/
// @__NO_SIDE_EFFECTS__
function getWitnessVersion(input: string): number {
const normalized = input.toLowerCase();
const separator = normalized.lastIndexOf('1');
return BECH32_CHARSET.indexOf(normalized[separator + 1]);
}
/**
* Checks if Bech32 data is a valid Bitcoin witness payload.
*
* @param data The Bech32 data.
*
* @returns Whether the Bech32 data is a valid Bitcoin witness payload.
*/
// @__NO_SIDE_EFFECTS__
function isBitcoinWitnessPayload(data: readonly number[]): boolean {
const program = convertBits(data.slice(1), 5, 8);
return !!(
program &&
program.length >= 2 &&
program.length <= 40 &&
(data[0] !== 0 || program.length === 20 || program.length === 32)
);
}
/**
* Checks if a string is a Bitcoin Bech32 or Bech32m address.
*
* @param input The string input.
*
* @returns Whether the string is a Bitcoin Bech32 or Bech32m address.
*/
// @__NO_SIDE_EFFECTS__
function isBtcAddressBech32(input: string): boolean {
if (!BTC_ADDRESS_BECH32_REGEX.test(input)) {
return false;
}
const witnessVersion = getWitnessVersion(input);
if (witnessVersion < 0 || witnessVersion > 16) {
return false;
}
return _isBech32(
input,
BITCOIN_BECH32_HRPS,
witnessVersion === 0 ? 'bech32' : 'bech32m',
isBitcoinWitnessPayload
);
}
📍 Affects 4 files
  • library/src/actions/btcAddress/btcAddress.ts#L13-L124 (this comment)
  • library/src/actions/btcAddressBech32/btcAddressBech32.ts#L12-L115
  • library/src/actions/btcAddressBase58/btcAddressBase58.ts#L12-L12
  • library/src/utils/_isBech32/_isBech32.ts#L9-L9
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@library/src/actions/btcAddress/btcAddress.ts` around lines 13 - 124, Remove
the duplicated constants and helpers from
library/src/actions/btcAddress/btcAddress.ts (13-124), instantiate
btcAddressBase58() and btcAddressBech32() once at module scope, and compose
btcAddress.requirement from their exported requirement functions. Keep
library/src/actions/btcAddressBech32/btcAddressBech32.ts (12-115) as the
canonical Bech32 implementation,
library/src/actions/btcAddressBase58/btcAddressBase58.ts (12) as the canonical
Base58 version source, and update library/src/utils/_isBech32/_isBech32.ts (9)
to export BECH32_CHARSET or an equivalent witness-version helper; these sibling
sites should provide reusable exports rather than copied logic.

@shuaixr
shuaixr force-pushed the feat-add-btc-address-action branch from 389feea to 7e7369f Compare July 19, 2026 09:11
@dosubot dosubot Bot added size:XL This PR changes 500-999 lines, ignoring generated files. and removed size:XXL This PR changes 1000+ lines, ignoring generated files. labels Jul 19, 2026
@yslpn

yslpn commented Jul 26, 2026

Copy link
Copy Markdown
Member

I don't think we can honestly verify a BTC address without using the sha256 function. This is quite dangerous, since users' funds and money depend on it.

We have a choice: add a dependency on noble with a synchronous function for working with sha256, or make the entire verification asynchronous using the web platform's browser API https://developer.mozilla.org/en-US/docs/Web/API/SubtleCrypto/digest

What do you think @fabian-hiller?

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

Labels

enhancement New feature or request size:XL This PR changes 500-999 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants