-
Notifications
You must be signed in to change notification settings - Fork 5
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.
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
mempool #118
base: main
Are you sure you want to change the base?
mempool #118
Conversation
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
crates/mempool/src/lib.rs
Outdated
type Tx; | ||
type Block; | ||
fn add_tx<S: Snapshot>(&mut self, snapshot: &S, tx: Self::Tx) -> bool; | ||
fn make_block<S: Snapshot>(&mut self, snapshot: &S) -> Option<Self::Block>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rather than thinking of this as make_block
, we should probably think of it as traversing the transactions in the mempool; We might want to traverse it for different purposes (answering the n2c mempool queries; etc.).
It just so happens that making a block wants to traverse transactions until it's filled up the block.
crates/mempool/src/lib.rs
Outdated
type Block; | ||
fn add_tx<S: Snapshot>(&mut self, snapshot: &S, tx: Self::Tx) -> bool; | ||
fn make_block<S: Snapshot>(&mut self, snapshot: &S) -> Option<Self::Block>; | ||
fn invalidate_utxos(&mut self, txins: HashSet<TransactionInput>); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
similarly, rather than thinking of this as invalidate_utxo
, we probably want to provide a new snapshot, or some delta to the last snapshot. For example, if a block gets minted that deregisters a stake address, that might invalidate a transaction that is trying to withdraw the staking rewards.
For performance reasons, we likely don't want to re-validate all of the existing transactions from scratch using the whole ledger snapshot, so that's why I said a delta, but conceptually that's what we're doing.
crates/mempool/src/lib.rs
Outdated
transactions: Vec<Tx>, | ||
} | ||
|
||
impl SimpleMempool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So that we can start exercising this code, make sure we're instantiating a mempool and calling invalidate_utxo
(or the snapshot-based equivalent) somewhere in the sync pipeline.
crates/mempool/src/lib.rs
Outdated
pub trait Mempool { | ||
type Tx; | ||
type Block; | ||
fn add_tx<S: Snapshot>(&mut self, snapshot: &S, tx: Self::Tx) -> bool; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
amaru_ledger::Snapshot
specifically refers to a frozen ledger snapshot related to a past epoch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should I use State
instead?
4fc2fab
to
2e8e15b
Compare
A
Mempool
trait rougly based on this writeup: #51 with a dummy implementation.