sephynox / xrpl-rust

A 100% Rust library to interact with the XRPL. XRPL Grant Winner
https://crates.io/crates/xrpl-rust
ISC License
20 stars 8 forks source link

Improve `Amount` and `Currency` #55

Closed LimpidCrypto closed 1 year ago

LimpidCrypto commented 1 year ago

High Level Overview of Change

Model conversion

This PR changes the lifetime of Currency and Amount from 'static to 'a. It also adds to_amount to turn a Currency into an Amount giving a value. Implement the From<Amount> for Currency trait to turn an Amount into a Currency.

value conversion

The get_value_as_u32 method was also replaced with TryInto<rust_decimal::Decimal> for Amount to have a more convenient way to deal with an Amount.value. The get_value_as_u32 also don't make sense for IssuedCurrencyAmount since it's value is most likely a decimal value.

Type of Change

Future Tasks

LimpidCrypto commented 1 year ago

Honestly I think it might be better if we had separate models for XRP and IssuedCurrency as currencies and XRPAmount and IssuedCurrencyAmount as amounts. That enables us to make our library to be stronger typed. Like an AccountRoot objects Balance could only be XRPAmount.

What's your opinion @sephynox ? If you agree I would close the PR and open a new PR soon.

LimpidCrypto commented 1 year ago

Honestly I think it might be better if we had separate models for XRP and IssuedCurrency as currencies and XRPAmount and IssuedCurrencyAmount as amounts. That enables us to make our library to be stronger typed. Like an AccountRoot objects Balance could only be XRPAmount.

What's your opinion @sephynox ? If you agree I would close the PR and open a new PR soon.

We agreed upon separating the currencies/amounts.