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/microblock fees #3144

Merged
merged 14 commits into from
Jun 7, 2022
+1,082 −60
Merged

Fix/microblock fees #3144

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
c0ad1e0
fix: fix #3140 by making it so that once the now-matured block's pare…
jcnelson May 19, 2022
6861ede
chore: plumb through the sortition DB handle to determine a matured r…
jcnelson May 19, 2022
18c6200
feat: add unit test coverage for both the old (incorrect) microblock …
jcnelson May 19, 2022
8463b20
Merge branch 'feat/btc-stx-exchange-rate' into fix/microblock-fees-3140
jcnelson May 20, 2022
8d6eb08
Merge branch 'feat/btc-stx-exchange-rate' into fix/microblock-fees-3140
jcnelson May 20, 2022
d1c3209
fix: update new unit tests so they don't stomp all over each other's …
jcnelson May 20, 2022
20cfd60
Merge branch 'feat/btc-stx-exchange-rate' into fix/microblock-fees-3140
jcnelson May 20, 2022
02f7401
Merge branch 'feat/btc-stx-exchange-rate' into fix/microblock-fees-3140
jcnelson May 20, 2022
e625081
chore: add unit test for the case where there are no microblocks, to …
jcnelson May 23, 2022
640f62e
Merge branch 'feat/btc-stx-exchange-rate' into fix/microblock-fees-3140
jcnelson May 23, 2022
6754379
fix: user-burn-supports don't get a coinbase reward for poison-microb…
jcnelson Jun 1, 2022
b1db0a0
fix: don't clobber another test's disk state
jcnelson Jun 6, 2022
8ed7199
chore: cargo fmt
jcnelson Jun 6, 2022
b33ad10
Merge branch 'feat/btc-stx-exchange-rate' into fix/microblock-fees-3140
jcnelson Jun 7, 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
38 changes: 31 additions & 7 deletions src/chainstate/stacks/db/accounts.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ use clarity::vm::types::*;

use crate::types::chainstate::{StacksAddress, StacksBlockId};

use crate::core::StacksEpochId;

#[derive(Debug, Clone, PartialEq)]
pub struct MinerReward {
pub address: StacksAddress,
Expand Down Expand Up @@ -873,6 +875,7 @@ impl StacksChainState {
/// not the miner, for reporting the microblock stream fork.
fn calculate_miner_reward(
mainnet: bool,
evaluated_epoch: StacksEpochId,
participant: &MinerPaymentSchedule,
miner: &MinerPaymentSchedule,
users: &Vec<MinerPaymentSchedule>,
Expand Down Expand Up @@ -946,7 +949,7 @@ impl StacksChainState {
)
} else {
// users that helped a miner that reported a poison-microblock get nothing
(StacksAddress::burn_address(mainnet), coinbase_reward, false)
(StacksAddress::burn_address(mainnet), 0, false)
}
} else {
// no poison microblock reported
Expand All @@ -963,12 +966,16 @@ impl StacksChainState {
} else {
0
},
// TODO: this is wrong, per #3140. It should be
// `participant.streamed_tx_fees_produced()`, since
// `participant.tx_fees_streamed` contains the sum of the microblock
// transaction fees that `participant` confirmed (and thus `participant`'s
// parent produced).
parent.streamed_tx_fees_produced(),
if evaluated_epoch < StacksEpochId::Epoch21 {
// this is wrong, per #3140. It should be
// `participant.streamed_tx_fees_produced()`, since
// `participant.tx_fees_streamed` contains the sum of the microblock
// transaction fees that `participant` confirmed (and thus `participant`'s
// parent produced). But we're stuck with it for earlier epochs.
parent.streamed_tx_fees_produced()
} else {
participant.streamed_tx_fees_produced()
},
if !punished {
participant.streamed_tx_fees_confirmed()
} else {
Expand Down Expand Up @@ -1015,6 +1022,7 @@ impl StacksChainState {
/// Returns a list of payments to make to each address -- miners and user-support burners.
pub fn find_mature_miner_rewards(
clarity_tx: &mut ClarityTx,
sortdb_conn: &Connection,
Copy link
Contributor

Choose a reason for hiding this comment

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

can unit tests for this function be used to test the miner reward calculations?

Copy link
Member Author

@jcnelson jcnelson Jun 1, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

They are, albeit indirectly, via test_get_block_info_v210 and test_get_block_info_v210_no_microblocks. Each block and microblock stream has a unique set of transaction fees, and the test verifies that these fees are awarded accordingly.

tip: &StacksHeaderInfo,
mut latest_matured_miners: Vec<MinerPaymentSchedule>,
parent_miner: MinerPaymentSchedule,
Expand Down Expand Up @@ -1043,6 +1051,15 @@ impl StacksChainState {
from_parent_block_consensus_hash: parent_miner.consensus_hash.clone(),
};

// what epoch was the parent miner's block evaluated in?
let evaluated_snapshot =
SortitionDB::get_block_snapshot_consensus(sortdb_conn, &parent_miner.consensus_hash)?
.expect("FATAL: no snapshot for evaluated block");

let evaluated_epoch =
SortitionDB::get_stacks_epoch(sortdb_conn, evaluated_snapshot.block_height)?
.expect("FATAL: no epoch for evaluated block");

// was this block penalized for mining a forked microblock stream?
// If so, find the principal that detected the poison, and reward them instead.
let poison_recipient_opt =
Expand All @@ -1062,6 +1079,7 @@ impl StacksChainState {
// calculate miner reward
let (parent_miner_reward, miner_reward) = StacksChainState::calculate_miner_reward(
mainnet,
evaluated_epoch.epoch_id,
&miner,
&miner,
&users,
Expand All @@ -1074,6 +1092,7 @@ impl StacksChainState {
for user_reward in users.iter() {
let (parent_reward, reward) = StacksChainState::calculate_miner_reward(
mainnet,
evaluated_epoch.epoch_id,
user_reward,
&miner,
&users,
Expand Down Expand Up @@ -1101,6 +1120,7 @@ mod test {
use crate::chainstate::stacks::index::*;
use crate::chainstate::stacks::Error;
use crate::chainstate::stacks::*;
use crate::core::StacksEpochId;
use clarity::vm::costs::ExecutionCost;
use stacks_common::util::hash::*;

Expand Down Expand Up @@ -1376,6 +1396,7 @@ mod test {

let (parent_reward, miner_reward) = StacksChainState::calculate_miner_reward(
false,
StacksEpochId::Epoch2_05,
&participant,
&participant,
&vec![],
Expand Down Expand Up @@ -1410,6 +1431,7 @@ mod test {

let (parent_miner_1, reward_miner_1) = StacksChainState::calculate_miner_reward(
false,
StacksEpochId::Epoch2_05,
&miner,
&miner,
&vec![user.clone()],
Expand All @@ -1418,6 +1440,7 @@ mod test {
);
let (parent_user_1, reward_user_1) = StacksChainState::calculate_miner_reward(
false,
StacksEpochId::Epoch2_05,
&user,
&miner,
&vec![user.clone()],
Expand Down Expand Up @@ -1458,6 +1481,7 @@ mod test {

let (parent_reward, miner_reward) = StacksChainState::calculate_miner_reward(
false,
StacksEpochId::Epoch2_05,
&participant,
&participant,
&vec![],
Expand Down
3 changes: 2 additions & 1 deletion src/chainstate/stacks/db/blocks.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4868,7 +4868,7 @@ impl StacksChainState {
chainstate_tx: &'b mut ChainstateTx,
clarity_instance: &'a mut ClarityInstance,
burn_dbconn: &'b dyn BurnStateDB,
conn: &Connection,
conn: &Connection, // connection to the sortition DB
chain_tip: &StacksHeaderInfo,
burn_tip: BurnchainHeaderHash,
burn_tip_height: u32,
Expand Down Expand Up @@ -4937,6 +4937,7 @@ impl StacksChainState {

let matured_miner_rewards_opt = match StacksChainState::find_mature_miner_rewards(
&mut clarity_tx,
conn,
&chain_tip,
latest_matured_miners,
matured_miner_parent,
Expand Down
Loading