XRPLF / xrpl-py

A Python library to interact with the XRP Ledger (XRPL) blockchain
ISC License
150 stars 84 forks source link

update the ISO currency code regular expression #674

Closed ckeshava closed 7 months ago

ckeshava commented 8 months ago

High Level Overview of Change

This PR updates the ISO Currency Code regex to better match the documentation.

Context of Change

Type of Change

Did you update CHANGELOG.md?

The PR adds a few unit tests to validate certain currency codes as inputs.

ckeshava commented 8 months ago

I'm not sure why the snippet tests are failing. I tested the send_escrow.py file on my local computer 12 times, it succeeded every time. I'll take a look again tomorrow.

ckeshava commented 8 months ago

Fixed

  • Currency codes with special characters not being allowed by IssuedCurrency objects.

ok 👍 @JST5000 Are you referring to the changelog file? the javascript codebase has a History.md file, the python codebase has the changelog file

ckeshava commented 8 months ago

Was this tested by hand to ensure that rippled supports it as well?

I haven't looked into the rippled code, I used the xrpl docs to verify this regex. Let me look into rippled as well

ckeshava commented 8 months ago

@mvadari I couldn't figure out how to do an end-to-end test on a rippled instance. But I verified these inputs as unit test cases against the rippled codebase and it works correctly. Here's the branch: https://github.com/ckeshava/rippled/tree/currCodeTests

Furthermore, here is the set of allowed characters in rippled: https://github.com/XRPLF/rippled/blob/87ee7868eacd4969b92981b4c93bfe63bd2af0be/src/ripple/protocol/impl/UintTypes.cpp#L35

JST5000 commented 8 months ago

Fixed

  • Currency codes with special characters not being allowed by IssuedCurrency objects.

ok 👍 @JST5000 Are you referring to the changelog file? the javascript codebase has a History.md file, the python codebase has the changelog file

Ah yep! I often get the name mixed up hahaha

mvadari commented 8 months ago

@mvadari I couldn't figure out how to do an end-to-end test on a rippled instance. But I verified these inputs as unit test cases against the rippled codebase and it works correctly. Here's the branch: https://github.com/ckeshava/rippled/tree/currCodeTests

Furthermore, here is the set of allowed characters in rippled: https://github.com/XRPLF/rippled/blob/87ee7868eacd4969b92981b4c93bfe63bd2af0be/src/ripple/protocol/impl/UintTypes.cpp#L35

You should test it via writing an integration test or writing a quick script to check.

ckeshava commented 7 months ago

@mvadari I have written integration tests to verify the new behavior.