Get order book changes

[LimpidCrypto]

High Level Overview of Change

• Add get_order_book_changes
• Add models for offer changes
• separate get_value and group_by_account to parser.py
• Add fields to Fields

Context of Change

The #342 PR was splitted. For each exported function (get_balance_changes (already merged #383), get_final_balances (already merged #398), get_previous_balances and get_order_book_changes) there will now be a separate PR to make it easier to review. This PR adds the functionality to parse the order book changes from a transaction's metadata (get_order_book_changes).

Reference is: https://github.com/ripple/ripple-lib-extensions/blob/master/transactionparser/src/orderbookchanges.js
The return value is structured as the get_balance_changes and get_final_balances return values.

Type of Change

• New feature (non-breaking change which adds functionality)
• Tests (You added tests for code that already exists, or your new feature included in this PR)

Test Plan

• test_offer_created
• test_offer_partially_filled_and_filled
• test_offer_cancelled
• test_offer_with_expiration

For Discussion

• When calculating the delta (_calculate_delta) the absolute value is returned. IMO it might be easier to understand if it is returned as a signed float ("value": "60" -> "value": "-60"), even though the values only can get smaller.
• Maybe there is a better name for arg account_objects in group_by_account.

TODO

• fix linter

[JST5000]

• When calculating the delta (_calculate_delta) the absolute value is returned. IMO it might be easier to understand if it is returned as a signed float ("value": "60" -> "value": "-60"), even though the values just can get smaller.

What do you mean by "the values just can get smaller"? (Would it always be negative in that case?)

[LimpidCrypto]

• When calculating the delta (_calculate_delta) the absolute value is returned. IMO it might be easier to understand if it is returned as a signed float ("value": "60" -> "value": "-60"), even though the values just can get smaller.

What do you mean by "the values just can get smaller"? (Would it always be negative in that case?)

Correct. An offer offer object can only be consumed or cancelled. Basically there are four statuses an offer can have:

• created = The transaction has created a new offer object.
• partially-filled = The transaction has consumed an offer object, for example for 50 %.
• filled = The transaction has consumed an offer object by 100 %.
• cancelled = The transaction has cancelled an offer with one of the statuses above.

To make it short, the taker amounts can not get raised by the owner, they can only get consumed (get smaller). Despite that it would make sense to explicitly indicate that the value got smaller, instead of always returning the absolute value, as the reference does.

[JST5000]

To make it short, the taker amounts can not get raised by the owner, they can only get consumed (get smaller). Despite that it would make sense to explicitly indicate that the value got smaller, instead of always returning the absolute value, as the reference does.

That makes sense to me - It does strike me as odd that a delta function which takes a final value and previous value would be absolute valued. (So, I'm happy if it just does the signed calculation, even if it happens to always be negative in this case)

[JST5000]

@LimpidCrypto Sorry for the slow review time - This week has been incredibly busy with travel, and I'm out on vacation next week - I'll make sure to prioritize this when I'm back a week from Tuesday though!

[LimpidCrypto]

@LimpidCrypto Sorry for the slow review time - This week has been incredibly busy with travel, and I'm out on vacation next week - I'll make sure to prioritize this when I'm back a week from Tuesday though!

No rush. Enjoy your well earned vacation :)

[JST5000]

I'll review this tomorrow :)

[JST5000]

LGTM (I asked for a second reviewer to look at this next week since tomorrow's an internal holiday for Ripplers :) )

[khancode]

LGTM. Just left a comment related to filename casing. Great work! 🤠

[LimpidCrypto]

How do I resolve the linting error 'poetry run flake8 xrpl tests --darglint-ignore-regex="^_(.*)"'?

[JST5000]

How do I resolve the linting error 'poetry run flake8 xrpl tests --darglint-ignore-regex="^_(.*)"'?

@LimpidCrypto Generally you want to run the following three commands:

1. poetry run black . - This will format the code according to https://pypi.org/project/black/
2. poetry run isort . - This sorts your imports so they're consistent
3. poetry run flake8 . - This reveals any other linting/style errors you may have (Each error should be searchable, and have a straightforward fix)

(The . in these is just saying to do it on all files, these should be run from the top level directory I believe)

(If you open up the details of the error you mentioned above, it'll show that almost all the errors are "BLK would make changes" which means poetry run black . should fix things :) )

[LimpidCrypto]

poetry run black . - This will format the code according to https://pypi.org/project/black/
poetry run isort . - This sorts your imports so they're consistent
poetry run flake8 . - This reveals any other linting/style errors you may have (Each error should be searchable, and have a straightforward fix)
(The . in these is just saying to do it on all files, these should be run from the top level directory I believe)
(If you open up the details of the error you mentioned above, it'll show that almost all the errors are "BLK would make changes" which means poetry run black . should fix things :) )

I tried that but still the same errors. It wonders me anyways because I did not even touch the files that cause these errors.

[JST5000]

I tried that but still the same errors. It wonders me anyways because I did not even touch the files that cause these errors.

Ok, I know the reason, but don't quite know the fix yet - It seems that the pre-commit hook is changing things back to a state that Black doesn't like automatically. (When you run poetry run black . it updates all the files correctly, but then once you try to commit that the A**B becomes A ** B which makes black unhappy again)

Currently looking into why this is happening :)

[JST5000]

@LimpidCrypto so seems that there's an update to either Black or flake8-black, so if you do poetry install it'll update those and then you can try poetry run black . and commit. I was able to get that working locally, so if all else fails I can push my local version, but hopefully that fixes the issue for you on your side :)

[LimpidCrypto]

@LimpidCrypto so seems that there's an update to either Black or flake8-black, so if you do poetry install it'll update those and then you can try poetry run black . and commit. I was able to get that working locally, so if all else fails I can push my local version, but hopefully that fixes the issue for you on your side :)

Great, thanks for your help @JST5000 :)