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

fix iter prefix #913

Merged
merged 18 commits into from
Feb 7, 2023
Merged

fix iter prefix #913

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
62ab0f5
core/storage: add a warning to `iter_prefix`
tzemanovic Dec 16, 2022
077291f
move write_log
brentstone Dec 25, 2022
a1284af
shared: Add WlStorage, combining WriteLog with Storage
tzemanovic Dec 16, 2022
59d69f4
write_log: add prefix iter
tzemanovic Dec 20, 2022
42d6df4
write_log: allow to commit genesis txs
tzemanovic Dec 22, 2022
6717108
storage: rename `fn commit` to `commit_block`
tzemanovic Dec 22, 2022
21ecedf
storage: extend iter_prefix test
tzemanovic Dec 22, 2022
b71e22b
test/vm_host_env: fix expected keys in VP iter prefix
tzemanovic Jan 11, 2023
326ebee
vp_host_fns: fix has_key_pre to see keys from prev txs (not yet in DB)
tzemanovic Jan 11, 2023
ff7e247
make -C wasm_for_tests/wasm_source
tzemanovic Jan 11, 2023
866338d
core: remove storage_api StorageRead and StorageWrite from Storage
tzemanovic Jan 11, 2023
bb7d173
use WlStorage in Shell and in queries RequestCtx
tzemanovic Jan 11, 2023
0f7779e
use storage PrefixIter instead of DB iterator that respects write log
tzemanovic Jan 11, 2023
9657c74
tests: update tests to use WlStorage
tzemanovic Jan 11, 2023
cd24137
fix IBC VP tests
yito88 Jan 11, 2023
e18afde
fix documentation
brentstone Jan 23, 2023
1bcfb71
changelog: add #913
tzemanovic Jan 12, 2023
c317e41
[ci] wasm checksums update
github-actions[bot] Feb 1, 2023
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
2 changes: 2 additions & 0 deletions .changelog/unreleased/bug-fixes/913-fix-iter-prefix.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- Fixed the prefix iterator method to respect modifications in the write log.
([#913](https://github.com/anoma/namada/pull/913))
7 changes: 4 additions & 3 deletions apps/src/lib/node/ledger/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,15 +58,16 @@ const ENV_VAR_RAYON_THREADS: &str = "NAMADA_RAYON_THREADS";
impl Shell {
fn load_proposals(&mut self) {
let proposals_key = gov_storage::get_commiting_proposals_prefix(
self.storage.last_epoch.0,
self.wl_storage.storage.last_epoch.0,
);

let (proposal_iter, _) = self.storage.iter_prefix(&proposals_key);
let (proposal_iter, _) =
self.wl_storage.storage.iter_prefix(&proposals_key);
for (key, _, _) in proposal_iter {
let key =
Key::from_str(key.as_str()).expect("Key should be parsable");
if gov_storage::get_commit_proposal_epoch(&key).unwrap()
!= self.storage.last_epoch.0
!= self.wl_storage.storage.last_epoch.0
{
// NOTE: `iter_prefix` iterate over the matching prefix. In this
// case a proposal with grace_epoch 110 will be
Expand Down
100 changes: 43 additions & 57 deletions apps/src/lib/node/ledger/shell/finalize_block.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

use namada::ledger::pos::types::into_tm_voting_power;
use namada::ledger::protocol;
use namada::ledger::storage::write_log::StorageModification;
use namada::ledger::storage_api::StorageRead;
use namada::types::storage::{BlockHash, BlockResults, Header};
use namada::types::token::Amount;
Expand Down Expand Up @@ -59,7 +58,7 @@ where
let mut stats = InternalStats::default();

// Tracks the accepted transactions
self.storage.block.results = BlockResults::default();
self.wl_storage.storage.block.results = BlockResults::default();
for (tx_index, processed_tx) in req.txs.iter().enumerate() {
let tx = if let Ok(tx) = Tx::try_from(processed_tx.tx.as_ref()) {
tx
Expand Down Expand Up @@ -129,7 +128,7 @@ where
// if the rejected tx was decrypted, remove it
// from the queue of txs to be processed
if let TxType::Decrypted(_) = &tx_type {
self.storage.tx_queue.pop();
self.wl_storage.storage.tx_queue.pop();
}
continue;
}
Expand All @@ -152,39 +151,16 @@ where

let balance_key =
token::balance_key(&wrapper.fee.token, &fee_payer);
let balance: Amount =
match self.write_log.read(&balance_key).0 {
Some(wal_mod) => {
// Read from WAL
if let StorageModification::Write { value } =
wal_mod
{
Amount::try_from_slice(value).unwrap()
} else {
Amount::default()
}
}
None => {
// Read from storage
let balance = StorageRead::read(
&self.storage,
&balance_key,
);
// Storage read must not fail, but there might
// be no value, in which
// case default (0) is returned
balance
.expect(
"Storage read in the protocol must \
not fail",
)
.unwrap_or_default()
}
};
let balance: token::Amount = self
.wl_storage
.read(&balance_key)
.expect("must be able to read")
.unwrap_or_default();

match balance.checked_sub(wrapper_fees) {
Some(amount) => {
self.write_log
self.wl_storage
.storage
.write(
&balance_key,
amount.try_to_vec().unwrap(),
Expand All @@ -198,7 +174,8 @@ where
let reject = true;
if reject {
// Burn remaining funds
self.write_log
self.wl_storage
.storage
.write(
&balance_key,
Amount::from(0).try_to_vec().unwrap(),
Expand All @@ -215,7 +192,7 @@ where
}
}

self.storage.tx_queue.push(WrapperTxInQueue {
self.wl_storage.storage.tx_queue.push(WrapperTxInQueue {
tx: wrapper.clone(),
#[cfg(not(feature = "mainnet"))]
has_valid_pow,
Expand All @@ -224,7 +201,7 @@ where
}
TxType::Decrypted(inner) => {
// We remove the corresponding wrapper tx from the queue
self.storage.tx_queue.pop();
self.wl_storage.storage.tx_queue.pop();
let mut event = Event::new_tx_event(&tx_type, height.0);

match inner {
Expand Down Expand Up @@ -271,8 +248,8 @@ where
.expect("transaction index out of bounds"),
),
&mut self.gas_meter,
&mut self.write_log,
&self.storage,
&mut self.wl_storage.write_log,
&self.wl_storage.storage,
&mut self.vp_wasm_cache,
&mut self.tx_wasm_cache,
)
Expand All @@ -287,10 +264,14 @@ where
result
);
stats.increment_successful_txs();
self.write_log.commit_tx();
self.wl_storage.commit_tx();
if !tx_event.contains_key("code") {
tx_event["code"] = ErrorCodes::Ok.into();
self.storage.block.results.accept(tx_index);
self.wl_storage
.storage
.block
.results
.accept(tx_index);
}
if let Some(ibc_event) = &result.ibc_event {
// Add the IBC event besides the tx_event
Expand Down Expand Up @@ -320,7 +301,7 @@ where
result.vps_result.rejected_vps
);
stats.increment_rejected_txs();
self.write_log.drop_tx();
self.wl_storage.drop_tx();
tx_event["code"] = ErrorCodes::InvalidTx.into();
}
tx_event["gas_used"] = result.gas_used.to_string();
Expand All @@ -333,7 +314,7 @@ where
msg
);
stats.increment_errored_txs();
self.write_log.drop_tx();
self.wl_storage.drop_tx();
tx_event["gas_used"] = self
.gas_meter
.get_current_transaction_gas()
Expand Down Expand Up @@ -382,22 +363,25 @@ where
hash: BlockHash,
byzantine_validators: Vec<Evidence>,
) -> (BlockHeight, bool) {
let height = self.storage.last_height + 1;
let height = self.wl_storage.storage.last_height + 1;

self.gas_meter.reset();

self.storage
self.wl_storage
.storage
.begin_block(hash, height)
.expect("Beginning a block shouldn't fail");

let header_time = header.time;
self.storage
self.wl_storage
.storage
.set_header(header)
.expect("Setting a header shouldn't fail");

self.byzantine_validators = byzantine_validators;

let new_epoch = self
.wl_storage
.storage
.update_epoch(height, header_time)
.expect("Must be able to update epoch");
Expand All @@ -410,11 +394,11 @@ where
/// changes to the validator sets and consensus parameters
fn update_epoch(&self, response: &mut shim::response::FinalizeBlock) {
// Apply validator set update
let (current_epoch, _gas) = self.storage.get_current_epoch();
let pos_params = self.storage.read_pos_params();
let (current_epoch, _gas) = self.wl_storage.storage.get_current_epoch();
let pos_params = self.wl_storage.read_pos_params();
// TODO ABCI validator updates on block H affects the validator set
// on block H+2, do we need to update a block earlier?
self.storage.validator_set_update(
self.wl_storage.validator_set_update(
current_epoch,
&pos_params,
|update| {
Expand Down Expand Up @@ -473,10 +457,11 @@ mod test_finalize_block {

// Add unshielded balance for fee paymenty
let balance_key = token::balance_key(
&shell.storage.native_token,
&shell.wl_storage.storage.native_token,
&Address::from(&keypair.ref_to()),
);
shell
.wl_storage
.storage
.write(&balance_key, Amount::whole(1000).try_to_vec().unwrap())
.unwrap();
Expand All @@ -490,7 +475,7 @@ mod test_finalize_block {
let wrapper = WrapperTx::new(
Fee {
amount: MIN_FEE.into(),
token: shell.storage.native_token.clone(),
token: shell.wl_storage.storage.native_token.clone(),
},
&keypair,
Epoch(0),
Expand Down Expand Up @@ -563,7 +548,7 @@ mod test_finalize_block {
let wrapper = WrapperTx::new(
Fee {
amount: 0.into(),
token: shell.storage.native_token.clone(),
token: shell.wl_storage.storage.native_token.clone(),
},
&keypair,
Epoch(0),
Expand Down Expand Up @@ -601,7 +586,7 @@ mod test_finalize_block {
assert_eq!(code, &String::from(ErrorCodes::InvalidTx));
}
// check that the corresponding wrapper tx was removed from the queue
assert!(shell.storage.tx_queue.is_empty());
assert!(shell.wl_storage.storage.tx_queue.is_empty());
}

/// Test that if a tx is undecryptable, it is applied
Expand All @@ -621,7 +606,7 @@ mod test_finalize_block {
let wrapper = WrapperTx {
fee: Fee {
amount: 0.into(),
token: shell.storage.native_token.clone(),
token: shell.wl_storage.storage.native_token.clone(),
},
pk: keypair.ref_to(),
epoch: Epoch(0),
Expand Down Expand Up @@ -659,7 +644,7 @@ mod test_finalize_block {
assert!(log.contains("Transaction could not be decrypted."))
}
// check that the corresponding wrapper tx was removed from the queue
assert!(shell.storage.tx_queue.is_empty());
assert!(shell.wl_storage.storage.tx_queue.is_empty());
}

/// Test that the wrapper txs are queued in the order they
Expand All @@ -674,10 +659,11 @@ mod test_finalize_block {

// Add unshielded balance for fee paymenty
let balance_key = token::balance_key(
&shell.storage.native_token,
&shell.wl_storage.storage.native_token,
&Address::from(&keypair.ref_to()),
);
shell
.wl_storage
.storage
.write(&balance_key, Amount::whole(1000).try_to_vec().unwrap())
.unwrap();
Expand All @@ -699,7 +685,7 @@ mod test_finalize_block {
let wrapper_tx = WrapperTx::new(
Fee {
amount: MIN_FEE.into(),
token: shell.storage.native_token.clone(),
token: shell.wl_storage.storage.native_token.clone(),
},
&keypair,
Epoch(0),
Expand Down Expand Up @@ -736,7 +722,7 @@ mod test_finalize_block {
let wrapper_tx = WrapperTx::new(
Fee {
amount: MIN_FEE.into(),
token: shell.storage.native_token.clone(),
token: shell.wl_storage.storage.native_token.clone(),
},
&keypair,
Epoch(0),
Expand Down
21 changes: 12 additions & 9 deletions apps/src/lib/node/ledger/shell/governance.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,10 @@ where
)
})?;

let votes = get_proposal_votes(&shell.storage, proposal_end_epoch, id);
let votes =
get_proposal_votes(&shell.wl_storage, proposal_end_epoch, id);
let is_accepted = votes.and_then(|votes| {
compute_tally(&shell.storage, proposal_end_epoch, votes)
compute_tally(&shell.wl_storage, proposal_end_epoch, votes)
});

let transfer_address = match is_accepted {
Expand Down Expand Up @@ -82,6 +83,7 @@ where
let pending_execution_key =
gov_storage::get_proposal_execution_key(id);
shell
.wl_storage
.storage
.write(&pending_execution_key, "")
.expect("Should be able to write to storage.");
Expand All @@ -92,19 +94,20 @@ where
* need it here. */
TxIndex::default(),
&mut BlockGasMeter::default(),
&mut shell.write_log,
&shell.storage,
&mut shell.wl_storage.write_log,
&shell.wl_storage.storage,
&mut shell.vp_wasm_cache,
&mut shell.tx_wasm_cache,
);
shell
.wl_storage
.storage
.delete(&pending_execution_key)
.expect("Should be able to delete the storage.");
match tx_result {
Ok(tx_result) => {
if tx_result.is_accepted() {
shell.write_log.commit_tx();
shell.wl_storage.write_log.commit_tx();
let proposal_event: Event =
ProposalEvent::new(
EventType::Proposal.to_string(),
Expand All @@ -119,7 +122,7 @@ where

proposal_author
} else {
shell.write_log.drop_tx();
shell.wl_storage.write_log.drop_tx();
let proposal_event: Event =
ProposalEvent::new(
EventType::Proposal.to_string(),
Expand All @@ -136,7 +139,7 @@ where
}
}
Err(_e) => {
shell.write_log.drop_tx();
shell.wl_storage.write_log.drop_tx();
let proposal_event: Event = ProposalEvent::new(
EventType::Proposal.to_string(),
TallyResult::Passed,
Expand Down Expand Up @@ -201,9 +204,9 @@ where
}
};

let native_token = shell.storage.native_token.clone();
let native_token = shell.wl_storage.storage.native_token.clone();
// transfer proposal locked funds
shell.storage.transfer(
shell.wl_storage.transfer(
&native_token,
funds,
&gov_address,
Expand Down
Loading