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 tx bond test condition #590

Merged
merged 4 commits into from
Oct 14, 2022
Merged

fix tx bond test condition #590

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1ec12c4
wasm/tx_bond: add test seed which makes it fail
tzemanovic Oct 13, 2022
d040923
wasm/tx_bond: fix delegation detection to compare source and validator
tzemanovic Oct 13, 2022
3136a10
changelog: add #590
tzemanovic Oct 13, 2022
6ea6baf
Apply suggestions from code review
tzemanovic Oct 13, 2022
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/testing/590-fix-tx-bond-test-condition.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- Fix a condition in tx_bond test that causes a false negative result
([#590](https://github.com/anoma/namada/pull/590))
1 change: 1 addition & 0 deletions wasm/wasm_source/proptest-regressions/tx_bond.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
cc e54347c5114ef29538127ba9ad68d1572af839ec63c015318fc0827818853a22
118 changes: 56 additions & 62 deletions wasm/wasm_source/src/tx_bond.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ mod tests {
key: key::common::SecretKey,
pos_params: PosParams,
) -> TxResult {
let is_delegation = matches!(
&bond.source, Some(source) if *source != bond.validator);
let staking_reward_address = address::testing::established_address_1();
let consensus_key = key::testing::keypair_1().ref_to();
let staking_reward_key = key::testing::keypair_2().ref_to();
Expand Down Expand Up @@ -154,71 +156,63 @@ mod tests {
source: bond_src,
};
let bonds_post = ctx().read_bond(&bond_id)?.unwrap();
match &bond.source {
Some(_) => {
// This bond was a delegation
for epoch in 0..pos_params.pipeline_len {
let bond: Option<Bond<token::Amount>> =
bonds_post.get(epoch);
assert!(
bond.is_none(),
"Delegation before pipeline offset should be empty - \
checking epoch {epoch}"
);
}
for epoch in pos_params.pipeline_len..=pos_params.unbonding_len
{
let start_epoch =
namada_tx_prelude::proof_of_stake::types::Epoch::from(
pos_params.pipeline_len,
);
let expected_bond =
HashMap::from_iter([(start_epoch, bond.amount)]);
let bond: Bond<token::Amount> =
bonds_post.get(epoch).unwrap();
assert_eq!(
bond.pos_deltas, expected_bond,
"Delegation at and after pipeline offset should be \
equal to the bonded amount - checking epoch {epoch}"
);
}

if is_delegation {
// A delegation is applied at pipeline offset
for epoch in 0..pos_params.pipeline_len {
let bond: Option<Bond<token::Amount>> = bonds_post.get(epoch);
assert!(
bond.is_none(),
"Delegation before pipeline offset should be empty - \
checking epoch {epoch}, got {bond:#?}"
);
}
None => {
let genesis_epoch =
namada_tx_prelude::proof_of_stake::types::Epoch::from(0);
// It was a self-bond
for epoch in 0..pos_params.pipeline_len {
let expected_bond =
HashMap::from_iter([(genesis_epoch, initial_stake)]);
let bond: Bond<token::Amount> =
bonds_post.get(epoch).expect(
"Genesis validator should already have self-bond",
);
assert_eq!(
bond.pos_deltas, expected_bond,
"Delegation before pipeline offset should be equal to \
the genesis initial stake - checking epoch {epoch}"
for epoch in pos_params.pipeline_len..=pos_params.unbonding_len {
let start_epoch =
namada_tx_prelude::proof_of_stake::types::Epoch::from(
pos_params.pipeline_len,
);
}
for epoch in pos_params.pipeline_len..=pos_params.unbonding_len
{
let start_epoch =
namada_tx_prelude::proof_of_stake::types::Epoch::from(
pos_params.pipeline_len,
);
let expected_bond = HashMap::from_iter([
(genesis_epoch, initial_stake),
(start_epoch, bond.amount),
]);
let bond: Bond<token::Amount> =
bonds_post.get(epoch).unwrap();
assert_eq!(
bond.pos_deltas, expected_bond,
"Delegation at and after pipeline offset should \
contain genesis stake and the bonded amount - \
checking epoch {epoch}"
let expected_bond =
HashMap::from_iter([(start_epoch, bond.amount)]);
let bond: Bond<token::Amount> = bonds_post.get(epoch).unwrap();
assert_eq!(
bond.pos_deltas, expected_bond,
"Delegation at and after pipeline offset should be equal \
to the bonded amount - checking epoch {epoch}"
);
}
} else {
let genesis_epoch =
namada_tx_prelude::proof_of_stake::types::Epoch::from(0);
// It was a self-bond
for epoch in 0..pos_params.pipeline_len {
let expected_bond =
HashMap::from_iter([(genesis_epoch, initial_stake)]);
let bond: Bond<token::Amount> = bonds_post
.get(epoch)
.expect("Genesis validator should already have self-bond");
assert_eq!(
bond.pos_deltas, expected_bond,
"Delegation before pipeline offset should be equal to the \
Copy link
Collaborator

Choose a reason for hiding this comment

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

Change assert failure message away from "Delegation" (perhaps "Self-bond")?

tzemanovic marked this conversation as resolved.
Show resolved Hide resolved
genesis initial stake - checking epoch {epoch}"
);
}
for epoch in pos_params.pipeline_len..=pos_params.unbonding_len {
let start_epoch =
namada_tx_prelude::proof_of_stake::types::Epoch::from(
pos_params.pipeline_len,
);
}
let expected_bond = HashMap::from_iter([
(genesis_epoch, initial_stake),
(start_epoch, bond.amount),
]);
let bond: Bond<token::Amount> = bonds_post.get(epoch).unwrap();
assert_eq!(
bond.pos_deltas, expected_bond,
"Delegation at and after pipeline offset should contain \
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as above regarding "Delegation"

tzemanovic marked this conversation as resolved.
Show resolved Hide resolved
genesis stake and the bonded amount - checking epoch \
{epoch}"
);
}
}

Expand Down