xrpl-py icon indicating copy to clipboard operation
xrpl-py copied to clipboard

`from_xrpl` should not accept snake_case dict keys

Open ckeshava opened this issue 1 year ago • 2 comments

BaseModel.from_xrpl method should not accept snake_case keys in dictionary input.

Fix #689 . You can find more context in the discussion pertaining to this issue.

I have used a regular expression matches for snake_case. If you have any other ideas for detecting non-camelCase input formats, let me know. There appears to be a variety of solutions for this problem, but all of them have slight differences in their behavior.

High Level Overview of Change

Context of Change

Type of Change

  • [ ] Bug fix (non-breaking change which fixes an issue)
  • [ ] New feature (non-breaking change which adds functionality)
  • [x] Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • [ ] Refactor (non-breaking change that only restructures code)
  • [x] Tests (You added tests for code that already exists, or your new feature included in this PR)
  • [ ] Documentation Updates
  • [ ] Release

Did you update CHANGELOG.md?

  • [ ] Yes
  • [x] No, this change does not impact library users Do external users use from_xrpl method? I believe they use from_dict (hasn't been modified) which internally makes a call to from_xrpl

Test Plan

ckeshava avatar Mar 06 '24 01:03 ckeshava

The integration and snippet tests are not reliable. I'm observing these errors: (Integration Test Python 3.8): xrpl.asyncio.wallet.wallet_generation.XRPLFaucetException: Unable to fund address with faucet after waiting 40 seconds

Snippet test error: httpx.HTTPStatusError: Server error '502 Bad Gateway' for url 'https://faucet.altnet.rippletest.net/accounts'

ckeshava avatar Mar 08 '24 01:03 ckeshava

@justinr1234 @khancode can you please review this at your convinience?

ckeshava avatar Mar 25 '24 19:03 ckeshava

I have updated the regular-expression which is used to invalidate inputs in BaseModel. This was required after the introduction of the AMM feature. (AMM introduced a new type of key Amount2, with an integer in its wake -- this was breaking the regexp)

If anyone has objections pertaining to it, please let me know. It is not related to the changelist of this PR.

ckeshava avatar Jun 21 '24 20:06 ckeshava