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 Bech32 and Bech32m Bitcoin Address Validation in Crypto Recognizer and expand tests #1307

Merged

Conversation

miltonsim
Copy link
Contributor

Change Description

  • Extended the CryptoRecognizer with the capability to validate Bech32 and Bech32m Bitcoin addresses
  • Added new test cases have been added to cover Bech32 and Bech32m address validation

Issue reference

This PR fixes issue #1306

Checklist

  • I have reviewed the contribution guidelines
  • I have signed the CLA (if required)
  • My code includes unit tests
  • All unit tests and lint checks pass locally
  • My PR contains documentation updates / additions if required

- Updated SG_NRIC_FIN Recognizer to include 'M' prefix for validating NRIC numbers issued to foreigners from 2022 onwards.
- Added new test cases to ensure comprehensive coverage and validation accuracy for all supported NRIC prefixes (S, T, F, G, M).
- Introduced a new test case to validate the recognition of valid NRIC/FIN numbers when embedded within a sentence
- Extended the CryptoRecognizer with the capability to validate Bech32 and Bech32m Bitcoin addresses
- Added new test cases have been added to cover Bech32 and Bech32m address validation
@omri374
Copy link
Contributor

omri374 commented Feb 20, 2024

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@omri374
Copy link
Contributor

omri374 commented Feb 20, 2024

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@omri374
Copy link
Contributor

omri374 commented Feb 20, 2024

Thanks!

@omri374 omri374 merged commit a8d2c90 into microsoft:main Feb 20, 2024
31 checks passed
@miltonsim
Copy link
Contributor Author

@omri374 thanks for merging my PR! I'm considering whether there might be interest in expanding the CryptoRecognizer to support addresses from cryptocurrencies other than Bitcoin. Would this be a valuable addition?

@omri374
Copy link
Contributor

omri374 commented Feb 21, 2024

Absolutely

@miltonsim
Copy link
Contributor Author

Cool! I'll look into it soon :)

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