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.

Sign up for GitHub

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

set_refund_for_current_tx #2361

Merged
merged 2 commits into from
Jul 2, 2024
Merged

set_refund_for_current_tx #2361

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e110d16
set_refund_for_current_tx
montekki Jul 1, 2024
aef3eb7
fixes from reviews
montekki Jul 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion core/lib/multivm/src/versions/shadow.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use std::{
fmt,
};

use pretty_assertions::assert_eq;
montekki marked this conversation as resolved.
Show resolved Hide resolved
use zksync_state::{ReadStorage, StoragePtr};
use zksync_types::{StorageKey, StorageLogWithPreviousValue, Transaction, U256};
use zksync_utils::bytecode::CompressedBytecodeInfo;
Expand Down Expand Up @@ -153,7 +154,7 @@ where
for (slot, value) in memory {
if *slot == 111 {
// FIXME: this particular memory slot (`OPERATOR_REFUNDS_OFFSET`) differs and is always zero for the new VM
*value = U256::zero();
//*value = U256::zero();
montekki marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
Expand Down
11 changes: 9 additions & 2 deletions core/lib/multivm/src/versions/vm_fast/vm.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ impl<S: ReadStorage> Vm<S> {
operator_suggested_refund: 0,
};
let mut last_tx_result = None;
let pubdata_before = self.inner.world_diff.pubdata.0 as u32;

let result = loop {
let hook = match self.inner.resume_from(self.suspended_at, &mut self.world) {
Expand Down Expand Up @@ -145,14 +146,15 @@ impl<S: ReadStorage> Vm<S> {
.as_u64();

let pubdata_published = self.inner.world_diff.pubdata.0 as u32;
//pubdata_published -= pubdata_before;
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove if this code isn't needed. BTW, how would pubdata computations work if the entire batch is executed (i.e., execution_mode == VmExecutionMode::Batch)? I'd think that pubdata_before needs to be updated somewhere in run() (in TxHasEnded hook handler?).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've changed it to be updated here on the spot, as for now i do not have a better theory about where it should be updated.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think it's correct (it would probably lead to the previous issue of pubdata_published for a transaction including pubdata produced by the bootloader / system contracts after executing the previous transaction), but let's fix this sequentially.


refund.operator_suggested_refund = compute_refund(
&self.batch_env,
bootloader_refund.as_u64(),
gas_spent_on_pubdata.as_u64(),
tx_gas_limit,
gas_per_pubdata_byte.low_u32(),
pubdata_published,
pubdata_published - pubdata_before,
self.bootloader_state
.last_l2_block()
.txs
Expand All @@ -161,10 +163,13 @@ impl<S: ReadStorage> Vm<S> {
.hash,
);

let refund_value = refund.operator_suggested_refund;
self.write_to_bootloader_heap([(
OPERATOR_REFUNDS_OFFSET + current_tx_index,
refund.operator_suggested_refund.into(),
refund_value.into(),
)]);
self.bootloader_state
.set_refund_for_current_tx(refund_value);
}
}
NotifyAboutRefund => {
Expand Down Expand Up @@ -310,6 +315,7 @@ impl<S: ReadStorage> Vm<S> {
/// Typically used to read the bootloader heap. We know that we're in the bootloader
/// when a hook occurs, as they are only enabled when preprocessing bootloader code.
pub(crate) fn read_heap_word(&self, word: usize) -> U256 {
// TODO: this should probably address `vm2::FIRST_HEAP` instead.
self.inner.state.heaps[self.inner.state.current_frame.heap].read_u256(word as u32 * 32)
}

Expand All @@ -318,6 +324,7 @@ impl<S: ReadStorage> Vm<S> {
memory: impl IntoIterator<Item = (usize, U256)>,
) {
assert!(self.inner.state.previous_frames.is_empty());
// TODO: this should probably address `vm2::FIRST_HEAP` instead.
let heap = &mut self.inner.state.heaps[self.inner.state.current_frame.heap];
for (slot, value) in memory {
heap.write_u256(slot as u32 * 32, value);
Expand Down
Loading