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

refactor(fee): remove more occurnces of resource mapping #547

Merged
merged 1 commit into from
Sep 5, 2024

Conversation

nimrod-starkware
Copy link
Contributor

@nimrod-starkware nimrod-starkware commented Aug 21, 2024

This change is Reviewable

@nimrod-starkware nimrod-starkware marked this pull request as ready for review August 21, 2024 08:32
@nimrod-starkware nimrod-starkware self-assigned this Aug 21, 2024
@nimrod-starkware nimrod-starkware force-pushed the nimrod/resources_enum/remove_possible_failure_of_tx_context_creation branch from 02f7ec8 to eec363c Compare August 21, 2024 08:40
@nimrod-starkware nimrod-starkware force-pushed the nimrod/resources_enum/remove_some_more branch from 9213b8f to 42998e5 Compare August 21, 2024 08:40
@nimrod-starkware nimrod-starkware force-pushed the nimrod/resources_enum/remove_possible_failure_of_tx_context_creation branch from eec363c to d1f5d45 Compare August 21, 2024 08:55
@nimrod-starkware nimrod-starkware force-pushed the nimrod/resources_enum/remove_some_more branch from 42998e5 to ac10dbc Compare August 21, 2024 08:55
@nimrod-starkware nimrod-starkware force-pushed the nimrod/resources_enum/remove_possible_failure_of_tx_context_creation branch from d1f5d45 to 4b7f6f4 Compare August 21, 2024 11:28
@nimrod-starkware nimrod-starkware force-pushed the nimrod/resources_enum/remove_some_more branch from ac10dbc to dbb4233 Compare August 21, 2024 11:28
@nimrod-starkware nimrod-starkware force-pushed the nimrod/resources_enum/remove_possible_failure_of_tx_context_creation branch from 4b7f6f4 to 995c2db Compare August 21, 2024 12:30
@nimrod-starkware nimrod-starkware force-pushed the nimrod/resources_enum/remove_some_more branch from dbb4233 to bd1181a Compare August 21, 2024 12:30
@nimrod-starkware nimrod-starkware force-pushed the nimrod/resources_enum/remove_possible_failure_of_tx_context_creation branch from 995c2db to 24d611b Compare August 21, 2024 12:33
@nimrod-starkware nimrod-starkware force-pushed the nimrod/resources_enum/remove_some_more branch from bd1181a to f3c04ae Compare August 21, 2024 12:33
@nimrod-starkware nimrod-starkware force-pushed the nimrod/resources_enum/remove_possible_failure_of_tx_context_creation branch from 24d611b to 2ce1278 Compare August 21, 2024 12:36
@nimrod-starkware nimrod-starkware force-pushed the nimrod/resources_enum/remove_some_more branch from f3c04ae to 85cf607 Compare August 21, 2024 12:36
Copy link
Collaborator

@dorimedini-starkware dorimedini-starkware left a comment

Choose a reason for hiding this comment

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

Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @nimrod-starkware and @ShahakShama)


crates/papyrus_storage/src/serialization/serializers.rs line 369 at r1 (raw file):

        pub max_amount: u64,
        pub max_price_per_unit: u128,
    }

@ShahakShama ?

@nimrod-starkware nimrod-starkware force-pushed the nimrod/resources_enum/remove_possible_failure_of_tx_context_creation branch from 2ce1278 to 2bf3583 Compare August 22, 2024 05:46
@nimrod-starkware nimrod-starkware force-pushed the nimrod/resources_enum/remove_some_more branch from 85cf607 to f92c17b Compare August 22, 2024 05:46
@nimrod-starkware nimrod-starkware force-pushed the nimrod/resources_enum/remove_possible_failure_of_tx_context_creation branch from 2bf3583 to c578a3f Compare August 22, 2024 06:04
@nimrod-starkware nimrod-starkware force-pushed the nimrod/resources_enum/remove_some_more branch from f92c17b to 0fc7eae Compare August 22, 2024 06:04
@nimrod-starkware nimrod-starkware force-pushed the nimrod/resources_enum/remove_possible_failure_of_tx_context_creation branch from c578a3f to 67d9f1c Compare August 22, 2024 09:06
@nimrod-starkware nimrod-starkware force-pushed the nimrod/resources_enum/remove_some_more branch from 0fc7eae to c27e3e9 Compare August 22, 2024 09:06
@nimrod-starkware nimrod-starkware force-pushed the nimrod/resources_enum/remove_possible_failure_of_tx_context_creation branch from 67d9f1c to 9eeb7ba Compare August 22, 2024 09:10
@nimrod-starkware nimrod-starkware force-pushed the nimrod/resources_enum/remove_some_more branch from c27e3e9 to 68faa71 Compare August 22, 2024 09:10
@nimrod-starkware nimrod-starkware force-pushed the nimrod/resources_enum/remove_possible_failure_of_tx_context_creation branch from 9eeb7ba to c7722e6 Compare August 22, 2024 09:36
@nimrod-starkware nimrod-starkware force-pushed the nimrod/resources_enum/remove_possible_failure_of_tx_context_creation branch from e122e41 to 69e2d09 Compare September 3, 2024 15:17
@nimrod-starkware nimrod-starkware force-pushed the nimrod/resources_enum/remove_some_more branch from 49c75ef to a1450e0 Compare September 3, 2024 15:17
Copy link
Collaborator

@dorimedini-starkware dorimedini-starkware left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r6, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @nimrod-starkware and @ShahakShama)

@nimrod-starkware nimrod-starkware force-pushed the nimrod/resources_enum/remove_possible_failure_of_tx_context_creation branch from 69e2d09 to deed29a Compare September 4, 2024 06:39
@nimrod-starkware nimrod-starkware force-pushed the nimrod/resources_enum/remove_some_more branch from a1450e0 to 3a77da9 Compare September 4, 2024 06:39
@nimrod-starkware nimrod-starkware force-pushed the nimrod/resources_enum/remove_possible_failure_of_tx_context_creation branch from deed29a to 33dc05a Compare September 4, 2024 07:04
@nimrod-starkware nimrod-starkware force-pushed the nimrod/resources_enum/remove_some_more branch from 3a77da9 to 2153557 Compare September 4, 2024 07:04
@nimrod-starkware nimrod-starkware force-pushed the nimrod/resources_enum/remove_possible_failure_of_tx_context_creation branch from 33dc05a to 0a4a144 Compare September 4, 2024 11:07
@nimrod-starkware nimrod-starkware force-pushed the nimrod/resources_enum/remove_some_more branch from 2153557 to ed50e99 Compare September 4, 2024 11:07
@nimrod-starkware nimrod-starkware force-pushed the nimrod/resources_enum/remove_possible_failure_of_tx_context_creation branch from 0a4a144 to 6aec949 Compare September 4, 2024 11:09
@nimrod-starkware nimrod-starkware force-pushed the nimrod/resources_enum/remove_some_more branch from ed50e99 to db8074f Compare September 4, 2024 11:09
Copy link

github-actions bot commented Sep 4, 2024

Benchmark movements:
tree_computation_flow performance regressed!
tree_computation_flow time: [66.816 ms 68.706 ms 71.124 ms]
change: [+1.2019% +3.8448% +7.9669%] (p = 0.01 < 0.05)
Performance has regressed.
Found 8 outliers among 100 measurements (8.00%)
2 (2.00%) high mild
6 (6.00%) high severe

@nimrod-starkware nimrod-starkware changed the base branch from nimrod/resources_enum/remove_possible_failure_of_tx_context_creation to graphite-base/547 September 4, 2024 13:23
Copy link

github-actions bot commented Sep 4, 2024

Benchmark movements:
tree_computation_flow performance improved 😺
tree_computation_flow time: [65.642 ms 65.932 ms 66.405 ms]
change: [-8.5613% -5.4521% -2.6476%] (p = 0.00 < 0.05)
Performance has improved.
Found 10 outliers among 100 measurements (10.00%)
5 (5.00%) high mild
5 (5.00%) high severe

@nimrod-starkware nimrod-starkware force-pushed the nimrod/resources_enum/remove_some_more branch from db8074f to 70b5a37 Compare September 4, 2024 13:34
@nimrod-starkware nimrod-starkware changed the base branch from graphite-base/547 to main September 4, 2024 13:35
@nimrod-starkware nimrod-starkware force-pushed the nimrod/resources_enum/remove_some_more branch from 70b5a37 to a64e601 Compare September 4, 2024 13:35
Copy link

github-actions bot commented Sep 4, 2024

Benchmark movements:
tree_computation_flow performance improved 😺
tree_computation_flow time: [65.702 ms 65.787 ms 65.884 ms]
change: [-8.0540% -4.8219% -2.1132%] (p = 0.00 < 0.05)
Performance has improved.
Found 4 outliers among 100 measurements (4.00%)
1 (1.00%) high mild
3 (3.00%) high severe

@nimrod-starkware nimrod-starkware force-pushed the nimrod/resources_enum/remove_some_more branch from a64e601 to f35a01b Compare September 5, 2024 07:17
@nimrod-starkware nimrod-starkware changed the title build(fee): remove more occurnces of resource mapping refactor(fee): remove more occurnces of resource mapping Sep 5, 2024
Copy link
Contributor

@ShahakShama ShahakShama left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 1 of 2 files at r7, all commit messages.
Reviewable status: 7 of 8 files reviewed, 1 unresolved discussion (waiting on @dorimedini-starkware and @nimrod-starkware)

Copy link
Contributor Author

@nimrod-starkware nimrod-starkware left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 2 files at r7.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @nimrod-starkware)

Copy link
Contributor Author

@nimrod-starkware nimrod-starkware left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @dorimedini-starkware)


crates/papyrus_storage/src/serialization/serializers.rs line 369 at r1 (raw file):

Previously, dorimedini-starkware wrote…

@ShahakShama ?

Done.

Copy link
Contributor Author

@nimrod-starkware nimrod-starkware left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 4 files at r1, 1 of 3 files at r2, 1 of 2 files at r3, 1 of 1 files at r5, 1 of 1 files at r6, 1 of 2 files at r7, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @dorimedini-starkware)

Copy link
Contributor Author

@nimrod-starkware nimrod-starkware left a comment

Choose a reason for hiding this comment

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

Dismissed @dorimedini-starkware from a discussion.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @nimrod-starkware)

@nimrod-starkware nimrod-starkware merged commit c65f0b9 into main Sep 5, 2024
54 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Sep 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants