Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[feature] #3660: Add Numeric type #4319

Merged
merged 10 commits into from
Mar 6, 2024

Conversation

Erigara
Copy link
Contributor

@Erigara Erigara commented Feb 22, 2024

Description

Introduce unified decimal numeric type for assets, no more Quantity, BigQuantity, Fixed.

Type is represented as 96bit mantissa + scale up to 28 decimal places.

Linked issue

Closes #3660

Benefits

Simplified code, decimals commonly use in monetary applications.

@Erigara Erigara added Enhancement New feature or request iroha2-dev The re-implementation of a BFT hyperledger in RUST labels Feb 22, 2024
@Erigara Erigara self-assigned this Feb 22, 2024
@github-actions github-actions bot added api-changes Changes in the API for client libraries config-changes Changes in configuration and start up of the Iroha labels Feb 22, 2024
Copy link

@BAStos525

@Erigara Erigara force-pushed the numeric branch 2 times, most recently from e5a271b to 82fe5b0 Compare February 22, 2024 12:54
@coveralls
Copy link

coveralls commented Feb 22, 2024

Pull Request Test Coverage Report for Build 8170013578

Details

  • 385 of 585 (65.81%) changed or added relevant lines in 25 files are covered.
  • 3515 unchanged lines in 61 files lost coverage.
  • Overall coverage decreased (-0.01%) to 56.774%

Changes Missing Coverage Covered Lines Changed/Added Lines %
core/src/smartcontracts/isi/mod.rs 0 1 0.0%
data_model/src/integer.rs 21 22 95.45%
data_model/src/domain.rs 3 5 60.0%
data_model/src/visit.rs 0 3 0.0%
smart_contract/executor/derive/src/default.rs 0 3 0.0%
data_model/src/predicate.rs 46 50 92.0%
smart_contract/executor/src/default.rs 0 6 0.0%
client_cli/src/main.rs 3 10 30.0%
core/src/wsv.rs 6 13 46.15%
data_model/src/lib.rs 4 11 36.36%
Files with Coverage Reduction New Missed Lines %
core/src/sumeragi/network_topology.rs 1 98.78%
tools/kagami/src/crypto.rs 1 12.12%
data_model/src/metadata.rs 2 78.87%
primitives/src/unique_vec.rs 2 91.24%
config/src/kura.rs 3 80.0%
data_model/src/asset.rs 4 48.81%
config/src/logger.rs 5 72.73%
logger/src/lib.rs 5 94.12%
tools/kagami/src/main.rs 5 0.0%
crypto/src/signature/secp256k1.rs 7 96.09%
Totals Coverage Status
Change from base Build 7884695009: -0.01%
Covered Lines: 22432
Relevant Lines: 39511

💛 - Coveralls

data_model/src/isi.rs Outdated Show resolved Hide resolved
@SamHSmith
Copy link
Contributor

I think this is very good. I currently have to expose this to library users and have them correctly match "Quantity" and "BigQuantity" et cetera. Having it all be the same type is much better.

SamHSmith
SamHSmith previously approved these changes Feb 23, 2024
primitives/numeric/Cargo.toml Show resolved Hide resolved
data_model/src/asset.rs Outdated Show resolved Hide resolved
core/src/smartcontracts/isi/account.rs Outdated Show resolved Hide resolved
data_model/src/integer.rs Show resolved Hide resolved
data_model/src/integer.rs Show resolved Hide resolved
primitives/numeric/Cargo.toml Outdated Show resolved Hide resolved
primitives/numeric/Cargo.toml Outdated Show resolved Hide resolved
primitives/numeric/src/lib.rs Outdated Show resolved Hide resolved
primitives/numeric/src/lib.rs Show resolved Hide resolved
primitives/numeric/src/lib.rs Outdated Show resolved Hide resolved
data_model/src/asset.rs Outdated Show resolved Hide resolved
mversic
mversic previously approved these changes Mar 4, 2024
client/tests/integration/asset.rs Show resolved Hide resolved
client/tests/integration/domain_owner_permissions.rs Outdated Show resolved Hide resolved
client/tests/integration/non_mintable.rs Outdated Show resolved Hide resolved
client/tests/integration/non_mintable.rs Outdated Show resolved Hide resolved
client/tests/integration/permissions.rs Show resolved Hide resolved
client/tests/integration/permissions.rs Show resolved Hide resolved
SamHSmith
SamHSmith previously approved these changes Mar 6, 2024
@Erigara Erigara dismissed stale reviews from SamHSmith and mversic via 4aa3817 March 6, 2024 09:46
@Erigara
Copy link
Contributor Author

Erigara commented Mar 6, 2024

pytest works fine locally
image

@Erigara Erigara merged commit 108084d into hyperledger-iroha:iroha2-dev Mar 6, 2024
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-changes Changes in the API for client libraries Enhancement New feature or request iroha2-dev The re-implementation of a BFT hyperledger in RUST
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants