refactor: make `AccountTransactionContext` hold BlockContext #1365

giladchase · 2024-01-22T13:53:53Z

Rename AccountTransactionContext into TransactionInfo: i want to
use Context for something else, and Account is a misnomer, since
L1Transactions also generate this instance.
Create a new AccountTransactionContext, which holds BlockContext
and TransactionInfo: This mirrors BlockContext, which holds
BlockInfo as well as higher level contexts.
Remove all unnecessary get_account_tx calls.
Replace all functions that take both block_context and tx_info with
a single TransactionContext.
Make EntryPointExecutionContext hold an Arc<TransactionContext>
instead of both a block_context and tx_info: previously every entry
point cloned the blockContext and generated a new tx_info, now they
all share the same (identical) one.

Python: https://reviewable.io/reviews/starkware-industries/starkware/33545

This change is

codecov-commenter · 2024-01-22T13:55:46Z

Codecov Report

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

Comparison is base (7a5c141) 70.74% compared to head (38af968) 70.51%.

Files	Patch %	Lines
.../blockifier/src/transaction/account_transaction.rs	85.54%	0 Missing and 12 partials ⚠️
crates/native_blockifier/src/py_validator.rs	0.00%	11 Missing ⚠️
...lockifier/src/execution/syscalls/hint_processor.rs	75.00%	0 Missing and 7 partials ⚠️
crates/blockifier/src/fee/fee_checks.rs	76.92%	3 Missing and 3 partials ⚠️
...lockifier/src/transaction/transaction_execution.rs	25.00%	5 Missing and 1 partial ⚠️
...ates/native_blockifier/src/transaction_executor.rs	0.00%	5 Missing ⚠️
crates/blockifier/src/execution/entry_point.rs	90.90%	0 Missing and 2 partials ⚠️
crates/blockifier/src/fee/actual_cost.rs	88.88%	0 Missing and 2 partials ⚠️
crates/blockifier/src/context.rs	90.00%	1 Missing ⚠️
crates/blockifier/src/fee/fee_utils.rs	95.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@               Coverage Diff                @@
##           main-v0.13.1    #1365      +/-   ##
================================================
- Coverage         70.74%   70.51%   -0.24%     
================================================
  Files                60       60              
  Lines              7862     7759     -103     
  Branches           7862     7759     -103     
================================================
- Hits               5562     5471      -91     
+ Misses             1871     1861      -10     
+ Partials            429      427       -2

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

elintul

Reviewed 1 of 23 files at r1, all commit messages.
Reviewable status: 1 of 23 files reviewed, all discussions resolved (waiting on @noaov1)

noaov1

Reviewed 5 of 23 files at r1, 2 of 7 files at r2, all commit messages.
Reviewable status: 7 of 23 files reviewed, 5 unresolved discussions (waiting on @elintul and @giladchase)

crates/blockifier/src/context.rs line 9 at r2 (raw file):

#[derive(Clone, Debug)]
pub struct TransactionContext {
    pub block_context: BlockContext,

Does it make sense?

Suggestion:

    pub block_context: Arc<BlockContext>,

crates/blockifier/src/context.rs line 41 at r2 (raw file):

    // TODO(Gilad): since fee_type is comes from TransactionInfo, we can make move this method into
    // TransactionContext, which has both the chain_info (through BlockContext) and the tx_info.
    // That is, add to BlockContext with the signature `pub fn fee_token_address(&self)`.

Suggestion:

    // TODO(Gilad): since fee_type comes from TransactionInfo, we can move this method into
    // TransactionContext, which has both the chain_info (through BlockContext) and the tx_info.
    // That is, add to BlockContext with the signature `pub fn fee_token_address(&self)`.

crates/blockifier/src/test_utils/struct_impls.rs line 42 at r2 (raw file):

    }

    pub fn execute_directly_given_account_context(

Suggestion:

pub fn execute_directly_given_tx_info(

crates/blockifier/src/test_utils/struct_impls.rs line 69 at r2 (raw file):

    }

    pub fn execute_directly_given_account_context_in_validate_mode(

Suggestion:

pub fn execute_directly_given_tx_info_in_validate_mode(

crates/blockifier/src/transaction/transactions.rs line 70 at r2 (raw file):

    }

    // FIXME: not transactional, maybe remove from trait.

Can you please elaborate?

Code quote:

    // FIXME: not transactional, maybe remove from trait.

elintul

Reviewed 13 of 23 files at r1, 6 of 7 files at r2, all commit messages.
Reviewable status: all files reviewed, 14 unresolved discussions (waiting on @giladchase)

crates/blockifier/src/execution/entry_point.rs line 144 at r2 (raw file):

#[derive(Debug)]
pub struct EntryPointExecutionContext {
    pub tx_context: Arc<TransactionContext>,

Consider shortly documenting the use of Arc.

Code quote:

pub tx_context: Arc<TransactionContext>

crates/blockifier/src/execution/entry_point.rs line 201 at r2 (raw file):

        limit_steps_by_resources: bool,
    ) -> TransactionExecutionResult<usize> {
        let block_context = &tx_context.block_context;

Suggestion:

let block_info = &tx_context.block_context.block_info;

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

    ) -> DeprecatedSyscallResult<Relocatable> {
        let tx_signature_start_ptr = self.get_or_allocate_tx_signature_segment(vm)?;
        let TransactionContext { block_context, tx_info } = self.context.tx_context.as_ref();

Coolio!
Did you unpack where possible?

Code quote:

let TransactionContext { block_context, tx_info }

crates/blockifier/src/execution/deprecated_syscalls/mod.rs line 399 at r2 (raw file):

    syscall_handler: &mut DeprecatedSyscallHintProcessor<'_>,
) -> DeprecatedSyscallResult<GetBlockNumberResponse> {
    // TODO(Yoni, 1/5/2024): disable for validate.

:(
Maybe start with getters on the hint processors?

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

        let (tx_signature_start_ptr, tx_signature_end_ptr) =
            &self.allocate_data_segment(vm, tx_info.signature().0)?;
        let tx_info = tx_info.clone();

Must we?

Code quote:

let tx_info = tx_info.clone();

crates/blockifier/src/fee/actual_cost.rs line 27 at r2 (raw file):

impl ActualCost {
    pub fn builder_for_l1_handler<'a>(

Why suddenly needed?

Code quote:

<'a>

crates/blockifier/src/fee/actual_cost.rs line 60 at r2 (raw file):

        Self {
            sender_address: Some(tx_context.tx_info.sender_address()),
            account_tx_context: tx_context,

Rename? Also additional such arguments.

Code quote:

account_tx_context

crates/blockifier/src/fee/fee_checks.rs line 91 at r2 (raw file):

        let ActualCost { actual_fee, actual_resources } = actual_cost;

        let versioned_constants = &tx_context.block_context.versioned_constants;

Suggestion:

        let ActualCost { actual_fee, actual_resources } = actual_cost;
        let versioned_constants = &tx_context.block_context.versioned_constants;

crates/blockifier/src/test_utils/struct_impls.rs line 48 at r2 (raw file):

        limit_steps_by_resources: bool,
    ) -> EntryPointExecutionResult<CallInfo> {
        let block_context = BlockContext::create_for_testing();

Pass directly, x2.

Code quote:

BlockContext::create_for_testing()

giladchase

Reviewable status: 20 of 23 files reviewed, 13 unresolved discussions (waiting on @elintul and @noaov1)

crates/blockifier/src/context.rs line 9 at r2 (raw file):

Previously, noaov1 (Noa Oved) wrote…

Does it make sense?

Good idea! But i'm wondering about how friendly it will be to force the API users to create an Arc 🤔 It's an implementation detail, which they might not support (no-std users come to mind).

How about we make BlockContext a trait that requires Clone implementation, and implement it as ArcBlockContext?
That way we'll get the Arc and users can also use it, but can also opt out by not using our Arc implementation, and instead use regular Clones.
(Everything will be documented of course.)

WDYT?

crates/blockifier/src/context.rs line 41 at r2 (raw file):

    // TODO(Gilad): since fee_type is comes from TransactionInfo, we can make move this method into
    // TransactionContext, which has both the chain_info (through BlockContext) and the tx_info.
    // That is, add to BlockContext with the signature `pub fn fee_token_address(&self)`.

Done.

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

Previously, elintul (Elin) wrote…

Coolio!
Did you unpack where possible?

Yep, up to this point, though more potential usages might free up when moving up the stack, i'll go over everything onces we're done and hunt for more usages (we already put a monday task for it)

giladchase

Reviewable status: 20 of 23 files reviewed, 12 unresolved discussions (waiting on @elintul and @noaov1)

crates/blockifier/src/execution/deprecated_syscalls/mod.rs line 399 at r2 (raw file):

Previously, elintul (Elin) wrote…

:(
Maybe start with getters on the hint processors?

Done

giladchase

Reviewable status: 18 of 23 files reviewed, 11 unresolved discussions (waiting on @elintul and @noaov1)

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

Previously, elintul (Elin) wrote…

Must we?

Done.

Had to change some stuff though to make this work without any clones (the only clone there is of tx_context, which is an Arc 💪 explanation below):
(I can undo and put in a separate PR, it's a bit out of scope here maybe?)

The clone() (of the Arc) is needed due to allocate_data_segment requiring &mut self: if we'd have defined

let tx_info = &self.context.tx_context.tx_info; // & is bound to self

then the reference will be bound to self (because it is a field on self), which would block allcoate_data_segment from taking &mut self. After the clone, we have:

let tx_info = &self.context.tx_context.clone().tx_info; // & is bound to the cloned tx_context.clone()

which bounds the & to the cloned tx_context, which does not prevent allocate_data_segment from &mutting self.

Made allocate_data_segment take &[T] instead of Vec<T>: ownership not needed, also &[T] is better than &Vec<T> due to deref coercion.
This allowed one to_vec call to be removed (saves an allocation).
The collect statement in the current module required an explicit Vec<StarkFelt> annotation: otherwise collect would type-infer &[StarkFelt], which is impossible there since this array is dynamically generated and it's size is not known at compile time. (This isn't a new allocation, previously it just type-inferred a Vec implicitly, before we changed the function signature). I added a PR comment there as well.

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

                ]
            })
            .collect();

Without type annotation it infers &[StarkFelt], which fails compilation due to this array being dynamically generated.
This is due to allocate_data_segment now requiring the more useful &[StarkFelt] over Vec<StarkFelt>.

Code quote:

        let flat_resource_bounds: Vec<StarkFelt> = context
            .resource_bounds
            .0
            .iter()
            })
            .collect();

elintul

Reviewed 1 of 23 files at r1, 3 of 3 files at r3, 1 of 2 files at r4, all commit messages.
Reviewable status: 22 of 23 files reviewed, 12 unresolved discussions (waiting on @giladchase and @noaov1)

crates/blockifier/src/execution/entry_point.rs line 145 at r4 (raw file):

pub struct EntryPointExecutionContext {
    // EntryPoints can recursively create other EntryPoints, we use `Arc` to avoid the clone of
    // this potentially large object.

Suggestion:

    // We use `Arc` to avoid the clone of this potentially large object, as inner calls
    // are created during execution.

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

Previously, giladchase wrote…

Yep, up to this point, though more potential usages might free up when moving up the stack, i'll go over everything onces we're done and hunt for more usages (we already put a monday task for it)

Thanks!

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

Previously, giladchase wrote…

Without type annotation it infers &[StarkFelt], which fails compilation due to this array being dynamically generated.
This is due to allocate_data_segment now requiring the more useful &[StarkFelt] over Vec<StarkFelt>.

Can you elaborate more? What changed here?

giladchase

Reviewable status: 22 of 23 files reviewed, 12 unresolved discussions (waiting on @elintul and @noaov1)

crates/blockifier/src/fee/actual_cost.rs line 27 at r2 (raw file):

Previously, elintul (Elin) wrote…

Why suddenly needed?

Because it's getting an argument by reference (the Arc) and returning an value that contains a reference of the same type (ActualCostBuilder holds this arc as well).
The compiler can't decide in such cases if the lifetime to fill instead of the '_ is the lifetime of the argument tx_context or the lifetime of the returned ActualCostBuilder itself (in our case it's the lifetime of the builder that matters).

Only half of this condition held previously: We did get BlockContext by reference as an argument, but ActualCostBuilder didn't hold a reference to it (only to other types, which weren't also passed as args).

crates/blockifier/src/fee/actual_cost.rs line 60 at r2 (raw file):

Previously, elintul (Elin) wrote…

Rename? Also additional such arguments.

Whoops i thought i caught all of them. Tnx!
Done (here and in fee_checks.rs).

giladchase

Reviewable status: 17 of 23 files reviewed, 11 unresolved discussions (waiting on @elintul and @noaov1)

crates/blockifier/src/test_utils/struct_impls.rs line 42 at r2 (raw file):

    }

    pub fn execute_directly_given_account_context(

Done.

crates/blockifier/src/test_utils/struct_impls.rs line 48 at r2 (raw file):

Previously, elintul (Elin) wrote…

Pass directly, x2.

To be sure i understand, you want me to pass blockContext from outside into this function?

This means adding a block_context fixture for every test that calls this function.

If we want to pass the entire tx_context from outside, it'll be an additional fixture + initializing the tx_context inside the test.

crates/blockifier/src/test_utils/struct_impls.rs line 69 at r2 (raw file):

    }

    pub fn execute_directly_given_account_context_in_validate_mode(

Done.

elintul

Reviewed 4 of 34 files at r10, 2 of 26 files at r11, all commit messages.
Reviewable status: 13 of 44 files reviewed, 5 unresolved discussions (waiting on @noaov1)

elintul

Reviewed 1 of 8 files at r12, all commit messages.
Reviewable status: 10 of 44 files reviewed, 6 unresolved discussions (waiting on @giladchase and @noaov1)

a discussion (no related file):
Bad rebase?

elintul

Reviewed 1 of 1 files at r8, 6 of 34 files at r10, 20 of 26 files at r11, 7 of 8 files at r12.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @giladchase and @noaov1)

elintul

Reviewed 3 of 3 files at r13, all commit messages.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @giladchase and @noaov1)

elintul

Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @giladchase and @noaov1)

giladchase

Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @elintul and @noaov1)

a discussion (no related file):

Previously, elintul (Elin) wrote…

Bad rebase?

Done.

elintul

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @noaov1)

noaov1

Reviewed 1 of 23 files at r1, 1 of 3 files at r3, 1 of 1 files at r8, 3 of 34 files at r10, 2 of 26 files at r11, 1 of 3 files at r13, all commit messages.
Reviewable status: 42 of 44 files reviewed, 2 unresolved discussions (waiting on @elintul and @giladchase)

crates/blockifier/src/context.rs line 9 at r2 (raw file):

Previously, giladchase wrote…

Good idea! But i'm wondering about how friendly it will be to force the API users to create an Arc 🤔 It's an implementation detail, which they might not support (no-std users come to mind).

How about we make BlockContext a trait that requires Clone implementation, and implement it as ArcBlockContext?
That way we'll get the Arc and users can also use it, but can also opt out by not using our Arc implementation, and instead use regular Clones.
(Everything will be documented of course.)

WDYT?

Sounds good. I think users also use EntryPointExecutionContext (that contains an Arc). Let's talk offline (when running "only" one transaction at a time it's not that significant, I guess)

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

        &mut self,
        vm: &mut VirtualMachine,
        context: &CurrentTransactionInfo,

Suggestion:

        tx_info: &CurrentTransactionInfo,

giladchase

Reviewable status: 41 of 44 files reviewed, 1 unresolved discussion (waiting on @elintul and @noaov1)

crates/blockifier/src/context.rs line 9 at r2 (raw file):

Previously, noaov1 (Noa Oved) wrote…

Sounds good. I think users also use EntryPointExecutionContext (that contains an Arc). Let's talk offline (when running "only" one transaction at a time it's not that significant, I guess)

How about i add it to monday in the concurrency board? we can talk about during the discussions then, since the assumption of "one tx at a time" will be broken soon...)

noaov1

Reviewed all commit messages.
Reviewable status: 41 of 44 files reviewed, all discussions resolved (waiting on @elintul)

noaov1

Reviewed 1 of 23 files at r1, 1 of 5 files at r5, 2 of 34 files at r10, 2 of 8 files at r12, 1 of 3 files at r13, 1 of 1 files at r15.
Reviewable status: 42 of 44 files reviewed, all discussions resolved (waiting on @elintul)

noaov1

Reviewed 1 of 2 files at r14.
Reviewable status: 43 of 44 files reviewed, 2 unresolved discussions (waiting on @elintul and @giladchase)

crates/blockifier/src/fee/fee_utils.rs line 98 at r15 (raw file):

    block_context: &BlockContext,
    fee_type: &FeeType,
) -> TransactionFeeResult<Fee> {

In another PR.

Suggestion:

pub fn calculate_tx_fee(
    resources: &ResourcesMapping,
    tx_context: &TransactionContext,
) -> TransactionFeeResult<Fee> {

crates/blockifier/src/fee/gas_usage.rs line 207 at r15 (raw file):

/// Return an estimated lower bound for the L1 gas on an account transaction.
pub fn estimate_minimal_gas_vector(
    tx_context: &TransactionContext,

Isn't block_context enough?

Code quote:

    tx_context: &TransactionContext,

noaov1

Reviewed 1 of 2 files at r14.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @elintul and @giladchase)

- Rename `AccountTransactionContext` into `TransactionInfo`: i want to use `Context` for something else, and `Account` is a misnomer, since L1Transactions also generate this instance. - Create a new `AccountTransactionContext`, which holds `BlockContext` and `TransactionInfo`: This mirrors `BlockContext`, which holds `BlockInfo` as well as higher level contexts. - Remove all unnecessary `get_account_tx` calls. - Replace all functions that take both `block_context` and `tx_info` with a single `TransactionContext`. - Make `EntryPointExecutionContext` hold an `Arc<TransactionContext>` instead of both a block_context and `tx_info`: previously every entry point cloned the blockContext and generated a new tx_info, now they all share the same (identical) one.

giladchase

Reviewable status: 39 of 44 files reviewed, 1 unresolved discussion (waiting on @elintul and @noaov1)

crates/blockifier/src/execution/entry_point.rs line 245 at r16 (raw file):

                    );
                    usize::MAX
                })

Looks like on main-v0.13.1 this test depends on this cast to be saturating in order to pass, so i had to do this in order to preserve the previous behavior.
@elintul should we task someone to investigate and fix this?

Code quote:

                // FIXME: This is saturating in the python bootstrapping test. Fix the value so
                // that it'll fit in a usize and remove the `as`.
                usize::try_from(max_cairo_steps).unwrap_or_else(|_| {
                    log::error!(
                        "Performed a saturating cast from u128 to usize: {max_cairo_steps:?}"
                    );
                    usize::MAX
                })

crates/blockifier/src/fee/fee_utils.rs line 98 at r15 (raw file):

Previously, noaov1 (Noa Oved) wrote…

In another PR.

Ya i started doing that, but then i hit a hardcoded fee typeso i let it go, i don't think i have runway to investigate it :(

crates/blockifier/src/fee/gas_usage.rs line 207 at r15 (raw file):

Previously, noaov1 (Noa Oved) wrote…

Isn't block_context enough?

Done

elintul

Reviewed 5 of 5 files at r16, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @giladchase and @noaov1)

crates/blockifier/src/fee/fee_utils.rs line 98 at r15 (raw file):

Previously, giladchase wrote…

Ya i started doing that, but then i hit a hardcoded fee typeso i let it go, i don't think i have runway to investigate it :(

Consider adding a TODO/Monday task.

elintul

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

giladchase

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

crates/blockifier/src/fee/fee_utils.rs line 98 at r15 (raw file):

Previously, elintul (Elin) wrote…

Consider adding a TODO/Monday task.

Done: https://starkware.monday.com/boards/5536734411/pulses/5994267610

giladchase force-pushed the gilad/rename-modules branch from e526b72 to 35cec60 Compare January 22, 2024 13:55

giladchase force-pushed the gilad/account-tx-info branch 2 times, most recently from fb1f499 to f49f42a Compare January 22, 2024 14:05

giladchase changed the title ~~Gilad/account tx info~~ refactor: make AccountTransactionContext hold BlockContext Jan 22, 2024

giladchase requested review from elintul and noaov1 January 22, 2024 14:35

giladchase force-pushed the gilad/rename-modules branch from 35cec60 to 1603e60 Compare January 22, 2024 17:59

giladchase force-pushed the gilad/account-tx-info branch from f49f42a to b63024a Compare January 22, 2024 18:00

giladchase force-pushed the gilad/rename-modules branch from 1603e60 to b429e75 Compare January 23, 2024 07:47

giladchase force-pushed the gilad/account-tx-info branch from b63024a to 3e2d1b9 Compare January 23, 2024 07:47

elintul reviewed Jan 23, 2024

View reviewed changes

giladchase force-pushed the gilad/rename-modules branch from b429e75 to 86c9ee7 Compare January 24, 2024 09:54

giladchase force-pushed the gilad/account-tx-info branch from 3e2d1b9 to 0c2e2ff Compare January 24, 2024 11:47

noaov1 suggested changes Jan 25, 2024

View reviewed changes

elintul suggested changes Jan 25, 2024

View reviewed changes

giladchase force-pushed the gilad/account-tx-info branch 2 times, most recently from 052adea to 9ab9e2f Compare January 28, 2024 07:20

giladchase commented Jan 28, 2024

View reviewed changes

giladchase force-pushed the gilad/account-tx-info branch 4 times, most recently from d8e1c79 to 078220f Compare January 28, 2024 07:25

giladchase commented Jan 28, 2024

View reviewed changes

elintul reviewed Jan 28, 2024

View reviewed changes

giladchase commented Jan 28, 2024

View reviewed changes

giladchase force-pushed the gilad/account-tx-info branch 2 times, most recently from ccee942 to 952adc1 Compare January 28, 2024 08:00

giladchase commented Jan 28, 2024

View reviewed changes

giladchase force-pushed the gilad/account-tx-info branch from ea76ec4 to a23cb93 Compare January 31, 2024 15:54

elintul reviewed Jan 31, 2024

View reviewed changes

Base automatically changed from gilad/rename-modules to main-v0.13.1 January 31, 2024 16:06

giladchase force-pushed the gilad/account-tx-info branch from a23cb93 to 09d12f8 Compare January 31, 2024 16:37

elintul suggested changes Jan 31, 2024

View reviewed changes

giladchase force-pushed the gilad/account-tx-info branch from 09d12f8 to 51afda5 Compare January 31, 2024 16:53

elintul reviewed Jan 31, 2024

View reviewed changes

elintul approved these changes Jan 31, 2024

View reviewed changes

giladchase commented Jan 31, 2024

View reviewed changes

elintul previously approved these changes Jan 31, 2024

View reviewed changes

giladchase force-pushed the gilad/account-tx-info branch from 51afda5 to 38af968 Compare February 1, 2024 04:49

noaov1 reviewed Feb 1, 2024

View reviewed changes

giladchase dismissed elintul’s stale review via d1a4fe0 February 1, 2024 10:56

giladchase force-pushed the gilad/account-tx-info branch from 38af968 to d1a4fe0 Compare February 1, 2024 10:56

giladchase commented Feb 1, 2024

View reviewed changes

noaov1 reviewed Feb 1, 2024

View reviewed changes

noaov1 requested a review from elintul February 1, 2024 12:03

noaov1 approved these changes Feb 1, 2024

View reviewed changes

noaov1 previously approved these changes Feb 1, 2024

View reviewed changes

giladchase dismissed noaov1’s stale review via e0a957e February 1, 2024 13:00

giladchase force-pushed the gilad/account-tx-info branch from d1a4fe0 to e0a957e Compare February 1, 2024 13:00

giladchase commented Feb 1, 2024

View reviewed changes

elintul approved these changes Feb 3, 2024

View reviewed changes

giladchase commented Feb 4, 2024

View reviewed changes

giladchase merged commit f76983c into main-v0.13.1 Feb 4, 2024
8 checks passed

giladchase deleted the gilad/account-tx-info branch February 4, 2024 10:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: make `AccountTransactionContext` hold BlockContext #1365

refactor: make `AccountTransactionContext` hold BlockContext #1365

giladchase commented Jan 22, 2024 •

edited

Loading

codecov-commenter commented Jan 22, 2024 •

edited

Loading

elintul left a comment

noaov1 left a comment

elintul left a comment

giladchase left a comment

giladchase left a comment

giladchase left a comment

elintul left a comment

giladchase left a comment

giladchase left a comment

elintul left a comment

elintul left a comment

elintul left a comment

elintul left a comment

elintul left a comment

giladchase left a comment

elintul left a comment

noaov1 left a comment

giladchase left a comment

noaov1 left a comment

noaov1 left a comment

noaov1 left a comment

noaov1 left a comment

giladchase left a comment

elintul left a comment

elintul left a comment

giladchase left a comment

refactor: make AccountTransactionContext hold BlockContext #1365

refactor: make AccountTransactionContext hold BlockContext #1365

Conversation

giladchase commented Jan 22, 2024 • edited Loading

codecov-commenter commented Jan 22, 2024 • edited Loading

Codecov Report

elintul left a comment

Choose a reason for hiding this comment

noaov1 left a comment

Choose a reason for hiding this comment

elintul left a comment

Choose a reason for hiding this comment

giladchase left a comment

Choose a reason for hiding this comment

giladchase left a comment

Choose a reason for hiding this comment

giladchase left a comment

Choose a reason for hiding this comment

elintul left a comment

Choose a reason for hiding this comment

giladchase left a comment

Choose a reason for hiding this comment

giladchase left a comment

Choose a reason for hiding this comment

elintul left a comment

Choose a reason for hiding this comment

elintul left a comment

Choose a reason for hiding this comment

elintul left a comment

Choose a reason for hiding this comment

elintul left a comment

Choose a reason for hiding this comment

elintul left a comment

Choose a reason for hiding this comment

giladchase left a comment

Choose a reason for hiding this comment

elintul left a comment

Choose a reason for hiding this comment

noaov1 left a comment

Choose a reason for hiding this comment

giladchase left a comment

Choose a reason for hiding this comment

noaov1 left a comment

Choose a reason for hiding this comment

noaov1 left a comment

Choose a reason for hiding this comment

noaov1 left a comment

Choose a reason for hiding this comment

noaov1 left a comment

Choose a reason for hiding this comment

giladchase left a comment

Choose a reason for hiding this comment

elintul left a comment

Choose a reason for hiding this comment

elintul left a comment

Choose a reason for hiding this comment

giladchase left a comment

Choose a reason for hiding this comment

refactor: make `AccountTransactionContext` hold BlockContext #1365

refactor: make `AccountTransactionContext` hold BlockContext #1365

giladchase commented Jan 22, 2024 •

edited

Loading

codecov-commenter commented Jan 22, 2024 •

edited

Loading