-
Notifications
You must be signed in to change notification settings - Fork 20.2k
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
internal: get pending and queued transaction by address #22992
internal: get pending and queued transaction by address #22992
Conversation
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.
This seems very wasteful to retrieve all that data just to throw away most of it. I think if you wanted to get thins functionality in, we should add a PendingFrom
and QueuedFrom
into the txpool where we can retrieve only the data we need.
Or to keep it consistent with te current API, |
6208b10
to
85050fa
Compare
@karalabe Thanks for your review. The PR is updated. |
internal/ethapi/api.go
Outdated
"pending": make(map[string]*RPCTransaction), | ||
"queued": make(map[string]*RPCTransaction), |
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.
You re-make
these later on, so these one just get overwritten
Looking at it a bit more closely, I think this is what Peter meant. When you call TxpoolContent, it goes down into this method: func (pool *TxPool) Content() (map[common.Address]types.Transactions, map[common.Address]types.Transactions) {
pool.mu.Lock()
defer pool.mu.Unlock()
pending := make(map[common.Address]types.Transactions)
for addr, list := range pool.pending {
pending[addr] = list.Flatten()
}
queued := make(map[common.Address]types.Transactions)
for addr, list := range pool.queue {
queued[addr] = list.Flatten()
}
return pending, queued
} You could instead define a new method which doesn't even bother with all the addresses that aren't what you're looking for, and save a lot of work. |
85050fa
to
d52be56
Compare
d52be56
to
82b7b36
Compare
@holiman, thanks for your review. I refined my PR based on your comments. :) |
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.
Looks good, thanks!
(caveat: I haven't tested it yet)
@karalabe Do you have any thoughts about this PR? |
1 similar comment
@karalabe Do you have any thoughts about this PR? |
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.
LGTM
* core, eth, internal, les, light: get pending and queued transaction by address * core: tiny nitpick fixes * light: tiny nitpick Co-authored-by: mark <[email protected]> Co-authored-by: Péter Szilágyi <[email protected]>
* core, eth, internal, les, light: get pending and queued transaction by address * core: tiny nitpick fixes * light: tiny nitpick Co-authored-by: mark <[email protected]> Co-authored-by: Péter Szilágyi <[email protected]>
We can call Content() to get
all
pending and queued transactions, but the response is really large...This pull request implements an API to filter the pending/queued transactions by an address.