Skip to content

Deny fee thieves - #2563

Open
Mirko-von-Leipzig wants to merge 2 commits into
nextfrom
mirko/deny-fee-eaters
Open

Deny fee thieves#2563
Mirko-von-Leipzig wants to merge 2 commits into
nextfrom
mirko/deny-fee-eaters

Conversation

@Mirko-von-Leipzig

Copy link
Copy Markdown
Collaborator

Summary

We are working on adding batch fee support. Fee notes from transactions in a batch should be consumed by the batch builder. Currently there is nothing preventing a new transaction a from consuming an unbatched fee note b' from transaction b, which will confuse the batch builder.

This PR therefore rejects transactions which attempt to consume a fee note from another inflight transaction. We purposefully allow consuming committed (but unconsumed) fee notes because these have already passed through the fee collection process, and are therefore open to anyone. Its unclear why this would occur; but perhaps there will be some purpose to these in the future.

Closes #2558.

Changelog

[[entry]]
scope       = "node"
impact      = "added"
description = "Transactions which consume fee notes are now rejected"

@igamigo igamigo left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM!

}

/// Returns the node that created the specified note.
pub(super) fn note_creator(&self, note: &miden_protocol::Word) -> Option<&N> {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Not related to this PR, but was there any reason why note identifiers within the mempool graph were typed as Word? Maybe because we used what used to be known as note commitment back then which didn't have a newtype. Would be nice for it to be NoteId now

return false;
};

!self.transaction_is_committed(creator) && is_fee_note(note)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit: Consider checking the note script first here to short-circuit the && since it's the cheaper one

Suggested change
!self.transaction_is_committed(creator) && is_fee_note(note)
is_fee_note(note) && !self.transaction_is_committed(creator)

Comment thread crates/block-producer/src/mempool/tests/add_transaction.rs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Reject transactions that consume fee notes

2 participants