build(fee): current tx_info holds validResourceBounds

[nimrod-starkware]

This change is 

[nimrod-starkware]

• refactor(fee): tx_args holds valid resource bounds #508 : 2 dependent PRs (#509 , #537 )
• refactor(fee): block context creator may fail #507
• refactor(fee): transaction info creator may fail #506
• build(fee): current tx_info holds validResourceBounds #505 👈
• refactor(fee): remove resource mapping usage in rpc and adjust to use all resources #580
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @nimrod-starkware and the rest of your teammates on Graphite

[nimrod-starkware]

Reviewable status: 0 of 5 files reviewed, 4 unresolved discussions (waiting on @dorimedini-starkware and @TzahiTaub)

---

crates/blockifier/src/execution/syscalls/hint_processor.rs line 480 at r1 (raw file):

                    l2_gas_as_felt,
                    Felt::ZERO,
                    Felt::ZERO,

WDYT?
l2_gas should always be zero.

Code quote:

                    l2_gas_as_felt,
                    Felt::ZERO,
                    Felt::ZERO,

---

crates/blockifier/src/transaction/transactions.rs line 299 at r1 (raw file):

                TransactionInfo::Current(CurrentTransactionInfo {
                    common_fields,
                    resource_bounds: tx.resource_bounds.0.clone().try_into().expect("todo"),

treated in an upper branch on the stack

Code quote:

try_into().expect("todo")

---

crates/blockifier/src/transaction/transactions.rs line 404 at r1 (raw file):

                TransactionInfo::Current(CurrentTransactionInfo {
                    common_fields,
                    resource_bounds: tx.resource_bounds.0.clone().try_into().expect("todo"),

ditto

Code quote:

0.clone().try_into().expect("todo"),

---

crates/blockifier/src/transaction/transactions.rs line 522 at r1 (raw file):

                TransactionInfo::Current(CurrentTransactionInfo {
                    common_fields,
                    resource_bounds: tx.resource_bounds.0.clone().try_into().expect("todo"),

ditto

Code quote:

.0.clone().try_into().expect("todo"),

[dorimedini-starkware]

Reviewed 2 of 5 files at r1, 3 of 3 files at r2, all commit messages.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @nimrod-starkware, @noaov1, and @TzahiTaub)

---

crates/blockifier/src/execution/syscalls/hint_processor.rs line 480 at r1 (raw file):

Previously, nimrod-starkware wrote…

WDYT?
l2_gas should always be zero.

I like it.
@noaov1 ?

---

crates/blockifier/src/execution/syscalls/hint_processor.rs line 209 at r2 (raw file):

pub const L2_GAS: &str = "0x00000000000000000000000000000000000000000000000000004c325f474153";
// "L1_DATA_GAS"
pub const L1_DATA_GAS: &str = "0x0000000000000000000000000000000000000000004c315f444154415f474153";

how did you compute this?
i see there is no test for these values... is it simple to add?

Code quote:

// "L1_DATA_GAS"
pub const L1_DATA_GAS: &str = "0x0000000000000000000000000000000000000000004c315f444154415f474153";

---

crates/blockifier/src/transaction/objects.rs line 142 at r2 (raw file):

        match self.resource_bounds {
            ValidResourceBounds::L1Gas(bounds) => Ok(bounds),
            ValidResourceBounds::AllResources { .. } => todo!(),

why todo? you can implement now

Suggestion:

 ValidResourceBounds::AllResources { l1_gas, .. } => l1_gas,

---

crates/blockifier/src/transaction/transactions.rs line 299 at r1 (raw file):

Previously, nimrod-starkware wrote…

treated in an upper branch on the stack

I think it's redundant... you can implement this in this PR,
however if this messes up your stack, LMK and I'll unblock

---

crates/blockifier/src/transaction/transactions.rs line 404 at r1 (raw file):

Previously, nimrod-starkware wrote…

ditto

ditto

---

crates/blockifier/src/transaction/transactions.rs line 522 at r1 (raw file):

Previously, nimrod-starkware wrote…

ditto

ditto

[dorimedini-starkware]

Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @nimrod-starkware, @noaov1, and @TzahiTaub)

---

crates/blockifier/src/transaction/objects.rs line 142 at r2 (raw file):

Previously, dorimedini-starkware wrote…

why todo? you can implement now

also, I see it isn't implemented at the top of the stack either

[dorimedini-starkware]

Reviewable status: all files reviewed, 7 unresolved discussions (waiting on @nimrod-starkware, @noaov1, and @TzahiTaub)

---

crates/blockifier/src/transaction/objects.rs line 144 at r2 (raw file):

            ValidResourceBounds::AllResources { .. } => todo!(),
        }
    }

this function always succeeds (or panics, but after implementing you will always succeed).
No need to return a Result

Suggestion:

    pub fn l1_resource_bounds(&self) -> ResourceBounds {
        match self.resource_bounds {
            ValidResourceBounds::L1Gas(bounds) => bounds,
            ValidResourceBounds::AllResources { .. } => todo!(),
        }
    }

[MohammadNassar1]

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @dorimedini-starkware, @nimrod-starkware, @noaov1, and @TzahiTaub)

---

crates/mempool_test_utils/src/starknet_api_test_utils.rs line 52 at r12 (raw file):

Previously, nimrod-starkware wrote…

I reverted VALID_L2_GAS_MAX_AMOUNT & VALID_L2_GAS_MAX_PRICE_PER_UNIT.
VALID_L1_DATA_GAS_MAX_AMOUNT & VALID_L1_DATA_GAS_MAX_PRICE_PER_UNIT is a copy paste.
IDK if it makes sense... @MohammadNassar1 ?

These are the values used for testing, specifically the L2 values that I recently added.
We used these values to establish resource bounds for testing. I believe no change is necessary here.

[dorimedini-starkware]

Reviewed 5 of 5 files at r14, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @MohammadNassar1, @nimrod-starkware, @noaov1, and @TzahiTaub)

[dorimedini-starkware]

Reviewed 3 of 3 files at r15, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @MohammadNassar1, @nimrod-starkware, @noaov1, and @TzahiTaub)

---

crates/blockifier/src/execution/syscalls/hint_processor.rs line 209 at r15 (raw file):

pub const L2_GAS: &str = "0x00000000000000000000000000000000000000000000000000004c325f474153";
// "L1_DATA";
pub const L1_DATA: &str = "0x000000000000000000000000000000000000000000000000004c315f44415441";

why drop the _GAS?

Code quote:

// "L1_DATA";
pub const L1_DATA: &str = "0x000000000000000000000000000000000000000000000000004c315f44415441";

[nimrod-starkware]

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @AvivYossef-starkware, @MohammadNassar1, @noaov1, and @TzahiTaub)

---

crates/blockifier/src/execution/syscalls/hint_processor.rs line 209 at r15 (raw file):

Previously, dorimedini-starkware wrote…

why drop the _GAS?

Discussed it with @AvivYossef-starkware .
L1_DATA_GAS introduces some overflow (he can explain better what that is)

[dorimedini-starkware]

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @AvivYossef-starkware, @MohammadNassar1, @nimrod-starkware, @noaov1, and @TzahiTaub)

---

crates/blockifier/src/execution/syscalls/hint_processor.rs line 209 at r15 (raw file):

Previously, nimrod-starkware wrote…

Discussed it with @AvivYossef-starkware .
L1_DATA_GAS introduces some overflow (he can explain better what that is)

@AvivYossef-starkware can you explain?

[dorimedini-starkware]

Reviewed 2 of 2 files at r16, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @AvivYossef-starkware, @MohammadNassar1, @nimrod-starkware, @noaov1, and @TzahiTaub)

[AvivYossef-starkware]

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @dorimedini-starkware, @MohammadNassar1, @nimrod-starkware, @noaov1, and @TzahiTaub)

---

crates/blockifier/src/execution/syscalls/hint_processor.rs line 209 at r15 (raw file):

Previously, dorimedini-starkware wrote…

@AvivYossef-starkware can you explain?

see here
https://github.com/EvyatarO/SNIPs/blob/snip-8/SNIPS/snip-8.md#:~:text=h(tip%2C%20resource_bounds_for_fee)%20%3D%20h(tip%2C%20(resource%7C%7Cmax_amount%7C%7Cmax_price_per_unit)%2C(resource%7C%7Cmax_amount%7C%7Cmax_price_per_unit)...)%2C%20where%20the%20resource%20order%20is%20L1_gas%2C%20L2_gas%2C%20and%20the%20resource%20name%20is%20at%20most%207%20characters.

[dorimedini-starkware]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @MohammadNassar1, @nimrod-starkware, @noaov1, and @TzahiTaub)

---

crates/blockifier/src/execution/syscalls/hint_processor.rs line 209 at r15 (raw file):

Previously, AvivYossef-starkware wrote…

see here
https://github.com/EvyatarO/SNIPs/blob/snip-8/SNIPS/snip-8.md#:~:text=h(tip%2C%20resource_bounds_for_fee)%20%3D%20h(tip%2C%20(resource%7C%7Cmax_amount%7C%7Cmax_price_per_unit)%2C(resource%7C%7Cmax_amount%7C%7Cmax_price_per_unit)...)%2C%20where%20the%20resource%20order%20is%20L1_gas%2C%20L2_gas%2C%20and%20the%20resource%20name%20is%20at%20most%207%20characters.

wow
hidden very well

[dorimedini-starkware]

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @TzahiTaub)

[dorimedini-starkware]

Reviewed 1 of 1 files at r17, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @TzahiTaub)

[nimrod-starkware]

Merge activity

• Aug 28, 9:25 AM EDT: @nimrod-starkware started a stack merge that includes this pull request via Graphite.
• Aug 28, 9:27 AM EDT: Graphite rebased this pull request as part of a merge.
• Aug 28, 9:44 AM EDT: @nimrod-starkware merged this pull request with Graphite.