Skip to content

Commit

Permalink
fix(consensus)!: add exhaust burn per transaction, fix block fee mall…
Browse files Browse the repository at this point in the history
…eability (#942)

Description
---
~~fix(consensus)!: calculate fees per block~~
fix(consensus)!: immalleable block fee 
feat(consensus)!: enable fixed exhaust of 5%

Motivation and Context
---
~~Previously, fees and exhaust burns were calculated per transaction.
This increases the deviation caused by rounding and isa little more
complex implementation-wise. This PR sums up fees for a block and then
computes the leader fee and exhaust burn.~~

The leader fee calculation is improved to better account for burnt fees
due to the remainder when dividing the fees by the number of involved
shards.

This PR enables the exhaust at a fixed rate of 5% - this number is not
finalized and will change but is enabled to allow for better testing.

The fee field in the block was previously malleable, this PR includes
the fees in the block hash, and therefore block signature.

How Has This Been Tested?
---
New unit test, manually checked that fee agreement is maintained

What process can a PR reviewer use to test or verify this change?
---
Check the expected fee and burn rate in VN database

Breaking Changes
---

- [ ] None
- [x] Requires data directory to be deleted
- [ ] Other - Please specify
  • Loading branch information
sdbondi committed Mar 12, 2024
1 parent de382d4 commit eb159a2
Show file tree
Hide file tree
Showing 23 changed files with 332 additions and 147 deletions.
Expand Up @@ -148,7 +148,7 @@ export default function BlockDetails() {
<TableCell>Total Fees</TableCell>
<DataTableCell>
<div className={block!.proposed_by === identity!.public_key ? "my_money" : ""}>
{block!.total_leader_fee}
{block!.block_fee.leader_fee}
</div>
</DataTableCell>
</TableRow>
Expand Down
Expand Up @@ -94,7 +94,7 @@ function Blocks() {
epoch: block.epoch,
height: block.height,
decision: block.justify.decision,
total_leader_fee: block.total_leader_fee,
total_leader_fee: block.block_fee.leader_fee,
proposed_by_me: block.proposed_by === identity.public_key,
transactions_cnt: block.commands.length,
block_time: times[block.id] - times[block.justify.block_id],
Expand Down
1 change: 1 addition & 0 deletions bindings/index.ts
Expand Up @@ -84,6 +84,7 @@ export * from "./src/types/ComponentAccessRules";
export * from "./src/types/SubstateDiff";
export * from "./src/types/AccessRule";
export * from "./src/types/VersionedSubstate";
export * from "./src/types/LeaderFee";
export * from "./src/types/ExecuteResult";
export * from "./src/types/OwnerRule";
export * from "./src/types/NonFungibleIndexAddress";
Expand Down
2 changes: 1 addition & 1 deletion bindings/src/types/Block.ts
Expand Up @@ -13,7 +13,7 @@ export interface Block {
height: NodeHeight;
epoch: Epoch;
proposed_by: string;
total_leader_fee: number;
total_leader_fee: bigint;
merkle_root: string;
commands: Array<Command>;
is_dummy: boolean;
Expand Down
6 changes: 6 additions & 0 deletions bindings/src/types/LeaderFee.ts
@@ -0,0 +1,6 @@
// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually.

export interface LeaderFee {
fee: number;
global_exhaust_burn: number;
}
3 changes: 2 additions & 1 deletion bindings/src/types/TransactionAtom.ts
@@ -1,11 +1,12 @@
// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually.
import type { Decision } from "./Decision";
import type { Evidence } from "./Evidence";
import type { LeaderFee } from "./LeaderFee";

export interface TransactionAtom {
id: string;
decision: Decision;
evidence: Evidence;
transaction_fee: number;
leader_fee: number;
leader_fee: LeaderFee | null;
}
1 change: 1 addition & 0 deletions clients/validator_node_client/src/types.rs
Expand Up @@ -461,6 +461,7 @@ pub struct GetCommitteeRequest {
pub struct GetCommitteeResponse {
pub committee: Committee<PeerAddress>,
}

#[derive(Debug, Clone, Serialize, Deserialize)]
#[cfg_attr(
feature = "ts",
Expand Down
3 changes: 1 addition & 2 deletions dan_layer/consensus/src/hotstuff/common.rs
Expand Up @@ -26,8 +26,7 @@ const LOG_TARGET: &str = "tari::dan::consensus::hotstuff::common";

/// The value that fees are divided by to determine the amount of fees to burn. 0 means no fees are burned.
/// This is a placeholder for the fee exhaust consensus constant so that we know where it's used later.
/// TODO: exhaust > 0
pub const EXHAUST_DIVISOR: u64 = 0;
pub const EXHAUST_DIVISOR: u64 = 20; // 5%

/// Calculates the dummy block required to reach the new height and returns the last dummy block (parent for next
/// proposal).
Expand Down
4 changes: 2 additions & 2 deletions dan_layer/consensus/src/hotstuff/on_propose.rs
Expand Up @@ -39,10 +39,10 @@ use tari_epoch_manager::EpochManagerReader;
use crate::{
hotstuff::{
calculate_state_merkle_diff,
common::EXHAUST_DIVISOR,
diff_to_substate_changes,
error::HotStuffError,
proposer,
EXHAUST_DIVISOR,
},
messages::{HotstuffMessage, ProposalMessage},
traits::{ConsensusSpec, OutboundMessaging, ValidatorSignatureService},
Expand Down Expand Up @@ -241,7 +241,7 @@ where TConsensusSpec: ConsensusSpec
))
})?;
let leader_fee = t.calculate_leader_fee(involved, EXHAUST_DIVISOR);
total_leader_fee += leader_fee;
total_leader_fee += leader_fee.fee();
let tx_atom = t.get_final_transaction_atom(leader_fee);
if tx_atom.decision.is_commit() {
let transaction = t.get_transaction(tx)?;
Expand Down
Expand Up @@ -41,7 +41,7 @@ use tokio::sync::broadcast;

use super::proposer::Proposer;
use crate::{
hotstuff::{common::EXHAUST_DIVISOR, error::HotStuffError, event::HotstuffEvent, ProposalValidationError},
hotstuff::{error::HotStuffError, event::HotstuffEvent, ProposalValidationError, EXHAUST_DIVISOR},
messages::{HotstuffMessage, VoteMessage},
traits::{
hooks::ConsensusHooks,
Expand Down Expand Up @@ -639,21 +639,32 @@ where TConsensusSpec: ConsensusSpec
block.id()
))
})?;

if t.leader_fee.is_none() {
warn!(
target: LOG_TARGET,
"❌ Leader fee for tx {} is None for Accept command in block {}",
t.id,
block.id(),
);
return Ok(None);
}

let calculated_leader_fee = tx_rec.calculate_leader_fee(distinct_shards, EXHAUST_DIVISOR);
if calculated_leader_fee != t.leader_fee {
if calculated_leader_fee != *t.leader_fee.as_ref().expect("None already checked") {
warn!(
target: LOG_TARGET,
"❌ Accept leader fee disagreement for block {}. Leader proposed {}, we calculated {}",
block.id(),
t.leader_fee,
t.leader_fee.as_ref().expect("None already checked"),
calculated_leader_fee
);

return Ok(None);
}
total_leader_fee += calculated_leader_fee;
total_leader_fee += calculated_leader_fee.fee();
// If the decision was changed to Abort, which can only happen when a foreign shard decides
// ABORT and we decide COMMIT, we set SomePrepared, otherwise
// ABORT, and we decide COMMIT, we set SomePrepared, otherwise
// AllPrepared. There are no further stages after these, so these MUST
// never be ready to propose.
if tx_rec.remote_decision().map(|d| d.is_abort()).unwrap_or(false) {
Expand Down Expand Up @@ -1085,7 +1096,6 @@ where TConsensusSpec: ConsensusSpec
.count(),
);
let mut total_transaction_fee = 0;
let mut total_fee_due = 0;
for cmd in block.commands() {
match cmd {
Command::Prepare(_t) => {},
Expand All @@ -1100,7 +1110,6 @@ where TConsensusSpec: ConsensusSpec
);

total_transaction_fee += tx_rec.transaction().transaction_fee;
total_fee_due += t.leader_fee;

let mut executed = t.get_executed_transaction(tx.deref_mut())?;
// Commit the transaction substate changes.
Expand Down Expand Up @@ -1158,9 +1167,9 @@ where TConsensusSpec: ConsensusSpec
if total_transaction_fee > 0 {
info!(
target: LOG_TARGET,
"🪙 Validator fee for block {} (amount due = {}, total fees = {})",
"🪙 Validator fee for block {} ({}, Total Fees Paid = {})",
block.proposed_by(),
total_fee_due,
block.total_leader_fee(),
total_transaction_fee
);
}
Expand Down
7 changes: 6 additions & 1 deletion dan_layer/p2p/proto/consensus.proto
Expand Up @@ -55,6 +55,11 @@ message Block {
bytes base_layer_block_hash = 14;
}

message LeaderFee {
uint64 leader_fee = 1;
uint64 global_exhaust_burn = 2;
}

message Command {
oneof command {
TransactionAtom prepare = 1;
Expand Down Expand Up @@ -84,7 +89,7 @@ message TransactionAtom {
Decision decision = 3;
Evidence evidence = 4;
uint64 fee = 5;
uint64 leader_fee = 6;
LeaderFee leader_fee = 6;
}

enum Decision {
Expand Down
26 changes: 24 additions & 2 deletions dan_layer/p2p/src/conversions/consensus.rs
Expand Up @@ -46,6 +46,7 @@ use tari_dan_storage::consensus_models::{
ForeignProposal,
ForeignProposalState,
HighQc,
LeaderFee,
QcId,
QuorumCertificate,
QuorumDecision,
Expand Down Expand Up @@ -341,7 +342,7 @@ impl From<&TransactionAtom> for proto::consensus::TransactionAtom {
decision: proto::consensus::Decision::from(value.decision) as i32,
evidence: Some((&value.evidence).into()),
fee: value.transaction_fee,
leader_fee: value.leader_fee,
leader_fee: value.leader_fee.as_ref().map(|a| a.into()),
}
}
}
Expand All @@ -360,7 +361,28 @@ impl TryFrom<proto::consensus::TransactionAtom> for TransactionAtom {
.ok_or_else(|| anyhow!("evidence not provided"))?
.try_into()?,
transaction_fee: value.fee,
leader_fee: value.leader_fee,
leader_fee: value.leader_fee.map(TryInto::try_into).transpose()?,
})
}
}
// -------------------------------- BlockFee -------------------------------- //

impl From<&LeaderFee> for proto::consensus::LeaderFee {
fn from(value: &LeaderFee) -> Self {
Self {
leader_fee: value.fee,
global_exhaust_burn: value.global_exhaust_burn,
}
}
}

impl TryFrom<proto::consensus::LeaderFee> for LeaderFee {
type Error = anyhow::Error;

fn try_from(value: proto::consensus::LeaderFee) -> Result<Self, Self::Error> {
Ok(Self {
fee: value.leader_fee,
global_exhaust_burn: value.global_exhaust_burn,
})
}
}
Expand Down
Expand Up @@ -183,20 +183,21 @@ create unique index transactions_uniq_idx_id on transactions (transaction_id);

create table transaction_pool
(
id integer not null primary key AUTOINCREMENT,
transaction_id text not null,
original_decision text not null,
local_decision text null,
remote_decision text null,
evidence text not null,
remote_evidence text null,
transaction_fee bigint not null,
leader_fee bigint not null,
stage text not null,
pending_stage text null,
is_ready boolean not null,
updated_at timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP,
created_at timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP,
id integer not null primary key AUTOINCREMENT,
transaction_id text not null,
original_decision text not null,
local_decision text null,
remote_decision text null,
evidence text not null,
remote_evidence text null,
transaction_fee bigint not null,
leader_fee bigint null,
global_exhaust_burn bigint null,
stage text not null,
pending_stage text null,
is_ready boolean not null,
updated_at timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP,
created_at timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP,
FOREIGN KEY (transaction_id) REFERENCES transactions (transaction_id)
);
create unique index transaction_pool_uniq_idx_transaction_id on transaction_pool (transaction_id);
Expand Down Expand Up @@ -310,22 +311,23 @@ CREATE UNIQUE INDEX pending_state_tree_diffs_uniq_idx_block_id on pending_state_
-- Debug Triggers
CREATE TABLE transaction_pool_history
(
history_id INTEGER PRIMARY KEY,
id integer not null,
transaction_id text not null,
original_decision text not null,
local_decision text null,
remote_decision text null,
evidence text not null,
transaction_fee bigint not null,
leader_fee bigint not null,
stage text not null,
new_stage text not null,
is_ready boolean not null,
new_is_ready boolean not null,
updated_at timestamp NOT NULL,
created_at timestamp NOT NULL,
change_time DATETIME DEFAULT (STRFTIME('%Y-%m-%d %H:%M:%f', 'NOW'))
history_id INTEGER PRIMARY KEY,
id integer not null,
transaction_id text not null,
original_decision text not null,
local_decision text null,
remote_decision text null,
evidence text not null,
transaction_fee bigint not null,
leader_fee bigint null,
global_exhaust_burn bigint null,
stage text not null,
new_stage text not null,
is_ready boolean not null,
new_is_ready boolean not null,
updated_at timestamp NOT NULL,
created_at timestamp NOT NULL,
change_time DATETIME DEFAULT (STRFTIME('%Y-%m-%d %H:%M:%f', 'NOW'))
);

CREATE TRIGGER copy_transaction_pool_history
Expand All @@ -341,6 +343,7 @@ BEGIN
evidence,
transaction_fee,
leader_fee,
global_exhaust_burn,
stage,
new_stage,
is_ready,
Expand All @@ -355,6 +358,7 @@ BEGIN
OLD.evidence,
OLD.transaction_fee,
OLD.leader_fee,
OLD.global_exhaust_burn,
OLD.stage,
NEW.stage,
OLD.is_ready,
Expand Down
42 changes: 21 additions & 21 deletions dan_layer/state_store_sqlite/src/reader.rs
Expand Up @@ -710,14 +710,14 @@ impl<'tx, TAddr: NodeAddressable + Serialize + DeserializeOwned + 'tx> StateStor
SELECT count(1) as "count" FROM tree WHERE bid = ? LIMIT 1
"#,
)
.bind::<Text, _>(serialize_hex(descendant))
// .bind::<Text, _>(serialize_hex(BlockId::genesis())) // stop recursing at zero block
.bind::<Text, _>(serialize_hex(ancestor))
.get_result::<Count>(self.connection())
.map_err(|e| SqliteStorageError::DieselError {
operation: "blocks_is_ancestor",
source: e,
})?;
.bind::<Text, _>(serialize_hex(descendant))
// .bind::<Text, _>(serialize_hex(BlockId::genesis())) // stop recursing at zero block
.bind::<Text, _>(serialize_hex(ancestor))
.get_result::<Count>(self.connection())
.map_err(|e| SqliteStorageError::DieselError {
operation: "blocks_is_ancestor",
source: e,
})?;

debug!(target: LOG_TARGET, "blocks_is_ancestor: is_ancestor: {}", is_ancestor.count);

Expand Down Expand Up @@ -1175,20 +1175,20 @@ impl<'tx, TAddr: NodeAddressable + Serialize + DeserializeOwned + 'tx> StateStor
// Are there any conflicts between this transaction and other transactions to be included in the
// block?
if processed_substates
.iter()
// Check other transactions
.filter(|(tx_id, _)| *tx_id != rec.transaction_id())
.all(|(_, evidence)| {
evidence.iter().all(|(shard, lock)| {
if lock.is_write() {
// Write lock must have no conflicts
!tx_substates.contains_key(shard)
} else {
// If there is a Shard conflict, then it must be a read lock
tx_substates.get(shard).map(|tx_lock| tx_lock.is_read()).unwrap_or(true)
}
})
.iter()
// Check other transactions
.filter(|(tx_id, _)| *tx_id != rec.transaction_id())
.all(|(_, evidence)| {
evidence.iter().all(|(shard, lock)| {
if lock.is_write() {
// Write lock must have no conflicts
!tx_substates.contains_key(shard)
} else {
// If there is a Shard conflict, then it must be a read lock
tx_substates.get(shard).map(|tx_lock| tx_lock.is_read()).unwrap_or(true)
}
})
})
{
used_substates.extend(tx_substates);
Some(Ok(rec))
Expand Down

0 comments on commit eb159a2

Please sign in to comment.