Skip to content
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

No need to add empty txns to subpool #30939

Open
yyuze opened this issue Dec 19, 2024 · 1 comment
Open

No need to add empty txns to subpool #30939

yyuze opened this issue Dec 19, 2024 · 1 comment
Labels

Comments

@yyuze
Copy link

yyuze commented Dec 19, 2024

System information

Geth version: geth version 1.14.11-stable-f3c696fa
Commit hash : f3c696fa1db75d0f78ea47dd0975f6f0de6fdd84

Coding issue

at core/txpool/txpool.go +338 would call p.subpools[i].Add even though txsets[i] is empty, it's not reasonable.

308 // Add enqueues a batch of transactions into the pool if they are valid. Due                                             
309 // to the large transaction churn, add may postpone fully integrating the tx                                             
310 // to a later point to batch multiple ones together.                                                                     
311 func (p *TxPool) Add(txs []*types.Transaction, local bool, sync bool) []error {                                          
312     // Split the input transactions between the subpools. It shouldn't really                                            
313     // happen that we receive merged batches, but better graceful than strange                                           
314     // errors.                                                                                                           
315     //                                                                                                                   
316     // We also need to track how the transactions were split across the subpools,                                        
317     // so we can piece back the returned errors into the original order.                                                 
318     txsets := make([][]*types.Transaction, len(p.subpools))                                                              
319     splits := make([]int, len(txs))                                                                                      
320                                                                                                                          
321     for i, tx := range txs {                                                                                             
322         // Mark this transaction belonging to no-subpool                                                                 
323         splits[i] = -1                                                                                                   
324                                                                                                                          
325         // Try to find a subpool that accepts the transaction                                                            
326         for j, subpool := range p.subpools {                                                                             
327             if subpool.Filter(tx) {                                                                                      
328                 txsets[j] = append(txsets[j], tx)                                                                        
329                 splits[i] = j                                                                                                                                                                                                                                              
330                 break                                                                                                    
331             }                                                                                                            
332         }                                                                                                                
333     }                                                                                                                    
334     // Add the transactions split apart to the individual subpools and piece                                             
335     // back the errors into the original sort order.                                                                     
336     errsets := make([][]error, len(p.subpools))                                                                          
337     for i := 0; i < len(p.subpools); i++ {                                                                               
338         errsets[i] = p.subpools[i].Add(txsets[i], local, sync)                                                           
339     }                                                                                                                    
340     errs := make([]error, len(txs))                                                                                      
341     for i, split := range splits {                                                                                       
342         // If the transaction was rejected by all subpools, mark it unsupported                                          
343         if split == -1 {                                                                                                 
344             errs[i] = core.ErrTxTypeNotSupported                                                                         
345             continue                                                                                                     
346         }                                                                                                                
347         // Find which subpool handled it and pull in the corresponding error                                             
348         errs[i] = errsets[split][0]                                                                                      
349         errsets[split] = errsets[split][1:]                                                                              
350     }                                                                                                                    
351     return errs                                                                                                          
352 }
@yyuze yyuze added the type:bug label Dec 19, 2024
@MariusVanDerWijden
Copy link
Member

This is not really a bug imo. There's nothing wrong happening here, the pools need to handle an empty transaction slice anyway, so I don't think this needs to be fixed, also there is no speed gained by fixing it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants