-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Add txpool TxInfo time submitted #275
Conversation
@ControlCplusControlV this adds time to txpool so it can be fetched from txpool for status info, and added Related to this: #263 |
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.
Both of these are very helpful, thank you! LGTM and with the way its implemented it looks like adding any additional information would be easy. Will refactor my changes to use this once merged
@@ -53,7 +54,7 @@ impl TxPool { | |||
} | |||
// check and insert dependency | |||
let rem = self.by_dependency.insert(&self.by_hash, db, &tx).await?; | |||
self.by_hash.insert(tx.id(), tx.clone()); | |||
self.by_hash.insert(tx.id(), TxInfo::new(tx.clone())); | |||
self.by_gas_price.insert(&tx); |
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.
[nit] Unrelated to the current PR, but should we normalize these secondary indexes to just map to the tx hash instead of arc's?
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.
hm, maybe, i wrap it around Arc
because i can clone it easily when i want to send list of transaction to block producer to use and i dont need to do deep copy, and second one is that it is used in multiple parts by_pair
and by_hash
No description provided.