XRPLF / xrpl-py

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

feat: Update `SetFee` with new syntax pending XRPFee amendment #634

Closed JST5000 closed 10 months ago

JST5000 commented 10 months ago

High Level Overview of Change

With the XRPFee amendment, the SetFee Psuedotransaction will have new fields.

Context of Change

Ideally, the old fields should still be usable up until the amendment is enabled, and for historical analysis of the ledger it should still be able to interpret older transactions as SetFee transactions. But we want to enforce that only one of the new syntaxes is allowed, not any combination of fields. (Meaning it should not be possible to instantiate a SetFee which has BOTH a base_fee AND a base_fee_drops)

Useful to see how this was done in xrpl.js: https://github.com/XRPLF/xrpl.js/pull/2357

Type of Change

Test Plan

CI Passes

JST5000 commented 10 months ago

Open to other suggestions about how to handle the typing.

My other ideas didn't pan out (Overloading constructors isn't done in Python, having a type alias named SetFee wouldn't be backwards compatible as it doesn't work as a constructor, and doing something fancy with multiple inheritance also didn't seem to work from my attempts because overloading constructors didn't work)

So, this implementation has the fields set as optional, but has explicit runtime checks and documentation to hopefully guide the user to picking the right fields if they're doing it by hand, and enforce the same "Required" rules the type checker normally would.