Skip to content

Fix governance bugs #2133

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

Merged
grarco merged 5 commits into from
Nov 11, 2023
Merged

Fix governance bugs #2133

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
29930bb
Fixes double mint in pgf
grarco Nov 8, 2023
693b560
Fixes `is_delegator` in governance vp
grarco Nov 8, 2023
475ef0e
Short circuits `is_delegator`
grarco Nov 9, 2023
035de7d
Fixes pgf stewards rewards minting
grarco Nov 9, 2023
201a836
Changelog #2133
grarco Nov 9, 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
7 changes: 4 additions & 3 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 @@ -830,22 +830,23 @@ where
pgf_fundings.sort_by(|a, b| a.id.cmp(&b.id));

for funding in pgf_fundings {
if credit_tokens(
if storage_api::token::transfer(
Copy link
Collaborator

@cwgoes cwgoes Nov 8, 2023

Choose a reason for hiding this comment

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

I don't think this is complete, we also need to update/change https://github.com/anoma/namada/blob/main/apps/src/lib/node/ledger/shell/finalize_block.rs#L858 - I think we can actually use credit_tokens there, but then we shouldn't send the steward inflation to the PGF account in the first place (https://github.com/anoma/namada/blob/main/apps/src/lib/node/ledger/shell/finalize_block.rs#L812)

&mut self.wl_storage,
&staking_token,
&pgf_address,
&funding.detail.target,
funding.detail.amount,
)
.is_ok()
{
tracing::info!(
"Minted {} tokens for {} project.",
"Paying {} tokens for {} project.",
funding.detail.amount.to_string_native(),
&funding.detail.target,
);
} else {
tracing::warn!(
"Failed Minting {} tokens for {} project.",
"Failed to pay {} tokens for {} project.",
funding.detail.amount.to_string_native(),
&funding.detail.target,
);
Expand Down
2 changes: 1 addition & 1 deletion proof_of_stake/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -734,7 +734,7 @@ pub fn is_delegator<S>(
epoch: Option<namada_core::types::storage::Epoch>,
) -> storage_api::Result<bool>
where
S: StorageRead + StorageWrite,
S: StorageRead,
{
let prefix = bonds_for_source_prefix(address);
match epoch {
Expand Down
18 changes: 5 additions & 13 deletions shared/src/ledger/governance/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -669,21 +669,13 @@ where
epoch: Epoch,
verifiers: &BTreeSet<Address>,
address: &Address,
delegation_address: &Address,
_delegation_address: &Address,
) -> Result<bool> {
let bond_handle = pos::namada_proof_of_stake::bond_handle(
Ok(pos::namada_proof_of_stake::is_delegator(
Copy link
Collaborator

@Fraccaman Fraccaman Nov 9, 2023

Choose a reason for hiding this comment

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

I think we could also add a check if address != delegation_address

&self.ctx.pre(),
address,
delegation_address,
);
let params =
pos::namada_proof_of_stake::read_pos_params(&self.ctx.pre())?;
let bond = bond_handle.get_sum(&self.ctx.pre(), epoch, &params)?;

if bond.is_some() && verifiers.contains(address) {
Ok(true)
} else {
Ok(false)
}
Some(epoch),
)? && verifiers.contains(address))
}
}

Expand Down