Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add btcAddress action #911

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

shuaixr
Copy link

@shuaixr shuaixr commented Nov 4, 2024

Perform checksum validation for a btc address

@fabian-hiller
Copy link
Owner

fabian-hiller commented Nov 4, 2024

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
Copy link
Author

shuaixr commented Nov 4, 2024

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
Copy link
Owner

fabian-hiller commented Nov 5, 2024

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
Copy link
Author

shuaixr commented Nov 5, 2024

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
Owner

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants