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

BE-239 - Include mint price denom among mint event attributes for minter-wl-flex #630

Merged
merged 4 commits into from
Jan 14, 2024

Conversation

MightOfOaks
Copy link
Member

@MightOfOaks MightOfOaks commented Dec 11, 2023

No description provided.

@MightOfOaks MightOfOaks changed the title Include mint price denom among mint event attributes for minter-wl-flex BE-239 - Include mint price denom among mint event attributes for minter-wl-flex Dec 11, 2023
Copy link

linear bot commented Dec 11, 2023

Copy link

codecov bot commented Dec 11, 2023

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (f4f6e82) 60.07% compared to head (b9c81d1) 60.07%.

Files Patch % Lines
...cts/minters/vending-minter-wl-flex/src/contract.rs 0.00% 4 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #630   +/-   ##
=======================================
  Coverage   60.07%   60.07%           
=======================================
  Files          83       83           
  Lines        4518     4518           
=======================================
  Hits         2714     2714           
  Misses       1804     1804           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

Cosm-Orc Gas Usage

Contract Op Name Gas Used Old Gas Used Gas Diff File
multiple_contracts Execute__minter_batch_exec_mint_token 3107256 18825195 -83.4942% e2e/src/tests/open_edition_factory_and_mint_tests.rs:272
Raw Report for b9c81d1
Contract Op Name Gas Used Gas Wanted File
vending_factory Instantiate__factory_inst 187243 257748 e2e/src/helpers/helper.rs:35
vending_factory Store__Store 3073903 4587710 e2e/src/helpers/chain.rs:78
vending_factory Execute__factory_exec_minter_inst 34037933 51033699 e2e/src/tests/factory_test.rs:98
vending_factory Execute__factory_exec_minter_inst_w_trading_time 3858140 5764011 e2e/src/tests/factory_test.rs:244
base_factory Store__Store 2688056 4008944 e2e/src/helpers/chain.rs:78
base_minter Store__Store 7008685 10489827 e2e/src/helpers/chain.rs:78
open_edition_factory Store__Store 3469044 5180426 e2e/src/helpers/chain.rs:78
open_edition_factory Execute__factory_exec_minter_inst 542175 790136 e2e/src/tests/open_edition_factory_and_mint_tests.rs:202
open_edition_factory Instantiate__factory_inst 186555 256715 e2e/src/helpers/open_edition_minter_helpers.rs:39
open_edition_factory Execute__factory_exec_minter_inst_w_trading_time 542889 791208 e2e/src/tests/open_edition_factory_and_mint_tests.rs:405
open_edition_minter Store__Store 8508482 12739523 e2e/src/helpers/chain.rs:78
sg721_base Store__Store 8395174 12569561 e2e/src/helpers/chain.rs:78
sg721_metadata_onchain Store__Store 8824616 13213724 e2e/src/helpers/chain.rs:78
sg721_nt Store__Store 7794873 11669109 e2e/src/helpers/chain.rs:78
sg721_updatable Store__Store 9181323 13748784 e2e/src/helpers/chain.rs:78
sg_whitelist Store__Store 3603321 5381841 e2e/src/helpers/chain.rs:78
sg_whitelist_flex Store__Store 3539283 5285784 e2e/src/helpers/chain.rs:78
vending_minter Store__Store 8839098 13235447 e2e/src/helpers/chain.rs:78
vending_minter_wl_flex Store__Store 9033422 13526933 e2e/src/helpers/chain.rs:78
whitelist_immutable Store__Store 2197332 3272858 e2e/src/helpers/chain.rs:78
minter Execute__minter_exec_mint_to_token 363993 522863 e2e/src/tests/open_edition_minter_executes_tests.rs:180
minter Execute__minter_exec_purge 139023 185466 e2e/src/tests/open_edition_minter_executes_tests.rs:281
minter Execute__minter_exec_update_per_addr_limit 209136 290585 e2e/src/tests/open_edition_minter_executes_tests.rs:216
minter Execute__minter_exec_update_trading_time 271323 383864 e2e/src/tests/open_edition_minter_executes_tests.rs:99
multiple_contracts Execute__minter_batch_exec_mint_token 3107256 4637997 e2e/src/tests/open_edition_factory_and_mint_tests.rs:272
multiple_contracts Execute__minter_batch_exec_mint_token_w_trading_time 1943161 2907510 e2e/src/tests/factory_test.rs:314

.add_attribute("seller_amount", seller_amount))
.add_attribute(
"network_fee",
coin(network_fee.u128(), mint_price.clone().denom).to_string(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need to change network fee if it is a coin already the to_string() implementation should be including amount and denom

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

network_fee is only a Uint128 here I am afraid.

.add_attribute("mint_price", mint_price.to_string())
.add_attribute(
"seller_amount",
coin(seller_amount.u128(), mint_price.denom).to_string(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here no need to do coin(..) again

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same for seller_amount, it's a Uint128.

"network_fee",
coin(network_fee.u128(), mint_price.clone().denom).to_string(),
)
.add_attribute("mint_price", mint_price.to_string())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks good although not sure if to_string is necessary

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Receiving a the trait From<Coin> is not implemented for std::string::String error otherwise.

@jhernandezb jhernandezb merged commit df0ffef into main Jan 14, 2024
19 of 23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants