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

fix panic when the rpc client is not initialized #11528

Merged
merged 6 commits into from
Oct 5, 2022

Conversation

krasi-georgiev
Copy link
Contributor

@krasi-georgiev krasi-georgiev commented Oct 1, 2022

What type of PR is this?

Bug fix

fixes this panic when the code tries to call the rpc client before it is initialized

[2022-09-26 07:03:54]  INFO initial-sync: Processing block batch of size 25 starting from  0xb89eddc1... 3960641/3972319 - estimated time remaining 2h35m42s blocksPerSecond=1.2 
peers=6                                                                                                                                                                          
panic: runtime error: invalid memory address or nil pointer dereference                                                                                                          
[signal SIGSEGV: segmentation violation code=0x1 addr=0x20 pc=0x23abfa6]                                                                                                         
                                                                                                                                                                                 
goroutine 326 [running]:                                                                                                                                                         
github.com/prysmaticlabs/prysm/v3/beacon-chain/execution.(*Service).NewPayload(0xc000012380, {0xbbad20?, 0xc046a836e0?}, {0xbcba90, 0xc01e95b040})                               
        beacon-chain/execution/engine_client.go:100 +0x246                                                                                                                       
github.com/prysmaticlabs/prysm/v3/beacon-chain/blockchain.(*Service).notifyNewPayload(0xc0001d0000, {0xbbad20?, 0xc023134f90?}, 0x2, 0xbc9e40?, {0xbc9e40, 0xc01d3c1b90})        
        beacon-chain/blockchain/execution_engine.go:215 +0x227                                                                                                                   
github.com/prysmaticlabs/prysm/v3/beacon-chain/blockchain.(*Service).onBlockBatch(0xc0001d0000, {0xbbad20?, 0xc023134f60?}, {0xc01a3261c0, 0x19, 0x0?}, {0xc023eb1c00, 0x19, 0x0?
})                                                                                                                                                                               
        beacon-chain/blockchain/process_block.go:416 +0x1212                                                                                                                     
github.com/prysmaticlabs/prysm/v3/beacon-chain/blockchain.(*Service).ReceiveBlockBatch(0xc0001d0000, {0xbbac78?, 0xc01a127440?}, {0xc01a3261c0, 0x19, 0x24}, {0xc023eb1c00, 0x19,
 0x19})                                                                                                                                                                          
        beacon-chain/blockchain/receive_block.go:93 +0x151                                                                                                                       
github.com/prysmaticlabs/prysm/v3/beacon-chain/sync/initial-sync.(*Service).processBatchedBlocks(0xc0133a8240, {0xbbac78, 0xc01a127440}, {0x186eca5?, 0x60?, 0x30d9400?}, {0xc01a
326000, 0x35, 0x40}, 0xc00d371630)                                                                                                                                               
        beacon-chain/sync/initial-sync/round_robin.go:288 +0x8e7                                                                                                                 
github.com/prysmaticlabs/prysm/v3/beacon-chain/sync/initial-sync.(*Service).processFetchedData(0xc01a125b90?, {0xbbac78?, 0xc01a127440?}, {0xc00d371680?, 0x0?, 0x30d9400?}, 0xc0
1a119548?, 0x0?)                                                                                                                                                                 
        beacon-chain/sync/initial-sync/round_robin.go:131 +0x11e                                                                                                                 
github.com/prysmaticlabs/prysm/v3/beacon-chain/sync/initial-sync.(*Service).syncToFinalizedEpoch(0xc0133a8240, {0xbbac78, 0xc01a127440}, {0x337f60?, 0x2ec4a30?, 0x30d9400?})    
        beacon-chain/sync/initial-sync/round_robin.go:84 +0x285                                                                                                                  
github.com/prysmaticlabs/prysm/v3/beacon-chain/sync/initial-sync.(*Service).roundRobinSync(0xc0133a8240, {0xe00000004?, 0xc000087ce0?, 0x30d9400?})                              
        beacon-chain/sync/initial-sync/round_robin.go:47 +0x187                                                                                                                  
github.com/prysmaticlabs/prysm/v3/beacon-chain/sync/initial-sync.(*Service).Start(0xc0133a8240)                                                                                  
        beacon-chain/sync/initial-sync/service.go:105 +0x597                                                                                                                     
created by github.com/prysmaticlabs/prysm/v3/runtime.(*ServiceRegistry).StartAll                                                                                                 
        runtime/service_registry.go:46 +0x210       

also reported in the flashbot repo
flashbots/prysm#7

@CLAassistant
Copy link

CLAassistant commented Oct 1, 2022

CLA assistant check
All committers have signed the CLA.

@prestonvanloon
Copy link
Member

Interesting approach to fixing this. Our typical solution is to add nil checks, but I am wondering why this client was not set in the constructor anyway.

cc: @rauljordan (#10498)

Raul, any thoughts here?

@nisdas
Copy link
Member

nisdas commented Oct 4, 2022

@prestonvanloon The reason it isn't added is because we need to create an active connection with the execution client. So doing this in the constructor seemed wrong.

@krasi-georgiev
Copy link
Contributor Author

krasi-georgiev commented Oct 4, 2022

What approach would you suggest?

@nisdas
Copy link
Member

nisdas commented Oct 4, 2022

@krasi-georgiev , your approach is fine. I was just explaining to preston why we cant create it in the constructor

beacon-chain/execution/service.go Outdated Show resolved Hide resolved
beacon-chain/execution/service.go Outdated Show resolved Hide resolved
beacon-chain/execution/service.go Outdated Show resolved Hide resolved
krasi-georgiev and others added 5 commits October 5, 2022 11:11
Co-authored-by: Nishant Das <nishdas93@gmail.com>
Co-authored-by: Nishant Das <nishdas93@gmail.com>
Co-authored-by: Nishant Das <nishdas93@gmail.com>
Signed-off-by: Krasi Georgiev <8903888+krasi-georgiev@users.noreply.github.com>
@rkapka rkapka merged commit c1446a3 into prysmaticlabs:develop Oct 5, 2022
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.

5 participants