[fix] #1845: Non-mintable assets can be minted once only. #2044

appetrosyan · 2022-03-31T06:32:19Z

Signed-off-by: Aleksandr Petrosyan [email protected]

Description of the Change

Non-mintable assets are now registered as Mintable::Once and can be minted precisely once.

Issue

Closes #1845

Benefits

Non-mintable assets are now functional.

Possible Drawbacks

None

Usage examples/tests

client/integration/non_mintable.rs

codecov · 2022-03-31T06:53:08Z

Codecov Report

Merging #2044 (eae4993) into iroha2-dev (2a94dba) will decrease coverage by 77.27%.
The diff coverage is n/a.

❗ Current head eae4993 differs from pull request most recent head a264c90. Consider uploading reports for the commit a264c90 to get more accurate results

@@              Coverage Diff               @@
##           iroha2-dev   #2044       +/-   ##
==============================================
- Coverage       77.27%       0   -77.28%     
==============================================
  Files             176       0      -176     
  Lines           24216       0    -24216     
==============================================
- Hits            18713       0    -18713     
+ Misses           5503       0     -5503

Impacted Files	Coverage Δ
cli/src/torii/routing.rs
cli/src/torii/tests.rs
client/examples/million_accounts_genesis.rs
client/src/client.rs
client/tests/integration/add_asset.rs
client/tests/integration/asset_propagation.rs
client/tests/integration/events/data.rs
client/tests/integration/events/pipeline.rs
...lient/tests/integration/multiple_blocks_created.rs
client/tests/integration/multisignature_account.rs
... and 166 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9889c65...a264c90. Read the comment docs.

client/tests/integration/non_mintable.rs

client/tests/integration/roles.rs

data_model/src/asset.rs

rust-toolchain.toml

data_model/src/asset.rs

data_model/src/events/data/events.rs

core/test_network/src/lib.rs

client/tests/integration/non_mintable.rs

client/tests/integration/roles.rs

core/src/smartcontracts/isi/query.rs

data_model/src/asset.rs

s8sato · 2022-04-08T15:02:13Z

client/tests/integration/non_mintable.rs

+    // However, this will fail
+    assert!(test_client
+        .poll_request(client::asset::by_account_id(account_id), |result| {
+            result.iter().any(|asset| {
+                asset.id().definition_id == asset_definition_id
+                    && *asset.value() == AssetValue::Quantity(400_u32)
+            })
+        })
+        .is_err());


Just a suggestion,
one of more explicit ways than waiting timeout would be to submit_blocking and query the rejected transaction and assert the rejection reason

data_model/src/asset.rs

Arjentix · 2022-04-11T19:45:20Z

data_model/src/asset.rs

+    }
+}
+
+impl From<(<AssetDefinition as Identifiable>::Id, AssetValueType, bool)> for AssetDefinition {


This thing looks a little bit strange for me

I would remove it as well. I think the given constructors are sufficient and this one would be unexpected to use. But I wouldn't be against implementing the following destructuring conversion:
impl From<AssetDefinition> for (<AssetDefinition as Identifiable>::Id, AssetValueType, bool)

data_model/src/asset.rs

mversic · 2022-04-13T08:06:20Z

data_model/src/asset.rs

+    }
+}
+
+impl From<(<AssetDefinition as Identifiable>::Id, AssetValueType, bool)> for AssetDefinition {


I would remove it as well. I think the given constructors are sufficient and this one would be unexpected to use. But I wouldn't be against implementing the following destructuring conversion:
impl From<AssetDefinition> for (<AssetDefinition as Identifiable>::Id, AssetValueType, bool)

data_model/src/asset.rs

…only. Signed-off-by: Aleksandr Petrosyan <[email protected]>

Signed-off-by: Aleksandr Petrosyan <[email protected]>

appetrosyan requested review from Arjentix, mversic and s8sato as code owners March 31, 2022 06:32

github-actions bot added the iroha2-dev The re-implementation of a BFT hyperledger in RUST label Mar 31, 2022

appetrosyan marked this pull request as draft March 31, 2022 06:33

appetrosyan force-pushed the i2-schema-endpoint branch 3 times, most recently from 0e989da to cf03236 Compare March 31, 2022 08:51

appetrosyan marked this pull request as ready for review April 1, 2022 11:23

appetrosyan force-pushed the i2-schema-endpoint branch 3 times, most recently from f70a616 to 4441696 Compare April 4, 2022 15:10

mversic requested changes Apr 4, 2022

View reviewed changes

appetrosyan force-pushed the i2-schema-endpoint branch from 4441696 to 4560d49 Compare April 4, 2022 19:17

Arjentix reviewed Apr 4, 2022

View reviewed changes

appetrosyan force-pushed the i2-schema-endpoint branch 2 times, most recently from fd5fcc7 to fe773b6 Compare April 6, 2022 07:16

appetrosyan requested review from Arjentix and mversic April 6, 2022 07:29

mversic reviewed Apr 6, 2022

View reviewed changes

core/src/smartcontracts/isi/query.rs Outdated Show resolved Hide resolved

data_model/src/asset.rs Show resolved Hide resolved

appetrosyan requested a review from mversic April 6, 2022 08:05

appetrosyan force-pushed the i2-schema-endpoint branch 2 times, most recently from 6c2f30b to e4de7b1 Compare April 7, 2022 07:24

mversic previously approved these changes Apr 7, 2022

View reviewed changes

appetrosyan dismissed mversic’s stale review via 882d78f April 7, 2022 07:54

appetrosyan force-pushed the i2-schema-endpoint branch from e4de7b1 to 882d78f Compare April 7, 2022 07:54

mversic self-requested a review April 7, 2022 07:56

mversic previously approved these changes Apr 7, 2022

View reviewed changes

s8sato previously approved these changes Apr 8, 2022

View reviewed changes

Arjentix previously approved these changes Apr 11, 2022

View reviewed changes

appetrosyan dismissed stale reviews from Arjentix, s8sato, and mversic via 93d8165 April 13, 2022 07:43

appetrosyan force-pushed the i2-schema-endpoint branch from 882d78f to 93d8165 Compare April 13, 2022 07:43

s8sato previously approved these changes Apr 13, 2022

View reviewed changes

mversic requested changes Apr 13, 2022

View reviewed changes

appetrosyan dismissed s8sato’s stale review via eae4993 April 13, 2022 08:21

appetrosyan force-pushed the i2-schema-endpoint branch from 93d8165 to eae4993 Compare April 13, 2022 08:21

mversic requested review from Arjentix, mversic and s8sato April 13, 2022 08:24

mversic previously approved these changes Apr 13, 2022

View reviewed changes

appetrosyan dismissed mversic’s stale review via 5aec46c April 13, 2022 08:25

appetrosyan force-pushed the i2-schema-endpoint branch from eae4993 to 5aec46c Compare April 13, 2022 08:25

appetrosyan requested a review from mversic April 13, 2022 08:26

Arjentix previously approved these changes Apr 13, 2022

View reviewed changes

mversic previously approved these changes Apr 13, 2022

View reviewed changes

appetrosyan dismissed stale reviews from mversic and Arjentix via dc1373c April 13, 2022 09:10

appetrosyan force-pushed the i2-schema-endpoint branch from 5aec46c to dc1373c Compare April 13, 2022 09:10

appetrosyan requested review from Arjentix and mversic April 13, 2022 09:10

appetrosyan added 3 commits April 13, 2022 13:11

[fix] hyperledger-iroha#1845: Non-mintable assets can be minted once …

354848c

…only. Signed-off-by: Aleksandr Petrosyan <[email protected]>

[test]: Add roles integration tests.

b1a7dc6

Signed-off-by: Aleksandr Petrosyan <[email protected]>

[refactor]: Add AssetDefinition builder.

a264c90

Signed-off-by: Aleksandr Petrosyan <[email protected]>

appetrosyan force-pushed the i2-schema-endpoint branch from dc1373c to a264c90 Compare April 13, 2022 09:11

Arjentix approved these changes Apr 13, 2022

View reviewed changes

mversic approved these changes Apr 13, 2022

View reviewed changes

appetrosyan merged commit 6781520 into hyperledger-iroha:iroha2-dev Apr 13, 2022

[fix] #1845: Non-mintable assets can be minted once only. #2044

[fix] #1845: Non-mintable assets can be minted once only. #2044

Uh oh!

Conversation

appetrosyan commented Mar 31, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of the Change

Issue

Benefits

Possible Drawbacks

Usage examples/tests

Uh oh!

codecov bot commented Mar 31, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

s8sato Apr 8, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Arjentix Apr 11, 2022

Choose a reason for hiding this comment

Uh oh!

mversic Apr 13, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mversic Apr 13, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

appetrosyan commented Mar 31, 2022 •

edited

Loading

codecov bot commented Mar 31, 2022 •

edited

Loading