Skip to content

debug system config #1179

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

debug system config #1179

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
341463e
debug system config consensus
yiweichi Apr 30, 2025
85ef933
debug system config consensus
yiweichi Apr 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions consensus/system_contract/system_contract.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,12 +89,14 @@ func (s *SystemContract) Start() {
}

func (s *SystemContract) fetchAddressFromL1() error {
address, err := s.client.StorageAt(s.ctx, s.config.SystemContractAddress, s.config.SystemContractSlot, nil)
_, err := s.client.StorageAt(s.ctx, s.config.SystemContractAddress, s.config.SystemContractSlot, nil)
if err != nil {
return fmt.Errorf("failed to get signer address from L1 System Contract: %w", err)
}
bAddress := common.BytesToAddress(address)

// bAddress := common.HexToAddress("0x756ea06bdee36de11f22dcca45a31d8a178ef3c6")
// bAddress := common.HexToAddress("0xD9E635beA2Ed2813eD1c550429FB965dEE58109B")
// bAddress := common.Address{}
bAddress := common.HexToAddress("0xfe3Ba6Fac4Fdd5C975A61089AF2a3508a86D6a8B")
Comment on lines +92 to +99
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Hardcoded address bypasses dynamic L1 storage resolution

This change fundamentally alters the system's behavior - the function still retrieves L1 storage data but then ignores it, instead assigning a hardcoded Ethereum address. This approach bypasses the dynamic address resolution mechanism that appears to be the original design intent.

Consider either:

  1. Properly using the retrieved storage value:
-	_, err := s.client.StorageAt(s.ctx, s.config.SystemContractAddress, s.config.SystemContractSlot, nil)
+	storage, err := s.client.StorageAt(s.ctx, s.config.SystemContractAddress, s.config.SystemContractSlot, nil)
 	if err != nil {
 		return fmt.Errorf("failed to get signer address from L1 System Contract: %w", err)
 	}
-	// bAddress := common.HexToAddress("0x756ea06bdee36de11f22dcca45a31d8a178ef3c6")
-	// bAddress := common.HexToAddress("0xD9E635beA2Ed2813eD1c550429FB965dEE58109B")
-	// bAddress := common.Address{}
-	bAddress := common.HexToAddress("0xfe3Ba6Fac4Fdd5C975A61089AF2a3508a86D6a8B")
+	bAddress := common.BytesToAddress(storage)
  1. If this change is intentional, modify the function to skip making the unnecessary L1 call entirely.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
_, err := s.client.StorageAt(s.ctx, s.config.SystemContractAddress, s.config.SystemContractSlot, nil)
if err != nil {
return fmt.Errorf("failed to get signer address from L1 System Contract: %w", err)
}
bAddress := common.BytesToAddress(address)
// bAddress := common.HexToAddress("0x756ea06bdee36de11f22dcca45a31d8a178ef3c6")
// bAddress := common.HexToAddress("0xD9E635beA2Ed2813eD1c550429FB965dEE58109B")
// bAddress := common.Address{}
bAddress := common.HexToAddress("0xfe3Ba6Fac4Fdd5C975A61089AF2a3508a86D6a8B")
storage, err := s.client.StorageAt(s.ctx, s.config.SystemContractAddress, s.config.SystemContractSlot, nil)
if err != nil {
return fmt.Errorf("failed to get signer address from L1 System Contract: %w", err)
}
bAddress := common.BytesToAddress(storage)

s.lock.Lock()
defer s.lock.Unlock()

Expand Down
12 changes: 6 additions & 6 deletions eth/handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -542,12 +542,12 @@ func (h *handler) BroadcastTransactions(txs types.Transactions) {
func (h *handler) minedBroadcastLoop() {
defer h.wg.Done()

for obj := range h.minedBlockSub.Chan() {
if ev, ok := obj.Data.(core.NewMinedBlockEvent); ok {
h.BroadcastBlock(ev.Block, true) // First propagate block to peers
h.BroadcastBlock(ev.Block, false) // Only then announce to the rest
}
}
// for obj := range h.minedBlockSub.Chan() {
// if ev, ok := obj.Data.(core.NewMinedBlockEvent); ok {
// h.BroadcastBlock(ev.Block, true) // First propagate block to peers
// h.BroadcastBlock(ev.Block, false) // Only then announce to the rest
// }
// }
Comment on lines +545 to +550
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Block broadcasting is completely disabled

This change completely disables the broadcasting of newly mined blocks to network peers. This will prevent nodes running this code from contributing to block propagation in the network, which could significantly impact network health and performance.

If this is temporary debugging code, please add a prominent comment explaining why it's disabled and when it should be re-enabled. If this is intended to be a permanent change, consider:

  1. Either removing the minedBroadcastLoop function entirely.
  2. Or explaining the rationale for disabling block broadcasting with clear comments.
func (h *handler) minedBroadcastLoop() {
	defer h.wg.Done()

+	// Block broadcasting disabled intentionally because:
+	// [EXPLANATION HERE]
	// for obj := range h.minedBlockSub.Chan() {
	// 	if ev, ok := obj.Data.(core.NewMinedBlockEvent); ok {
	// 		h.BroadcastBlock(ev.Block, true)  // First propagate block to peers
	// 		h.BroadcastBlock(ev.Block, false) // Only then announce to the rest
	// 	}
	// }
}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// for obj := range h.minedBlockSub.Chan() {
// if ev, ok := obj.Data.(core.NewMinedBlockEvent); ok {
// h.BroadcastBlock(ev.Block, true) // First propagate block to peers
// h.BroadcastBlock(ev.Block, false) // Only then announce to the rest
// }
// }
func (h *handler) minedBroadcastLoop() {
defer h.wg.Done()
// Block broadcasting disabled intentionally because:
// [EXPLANATION HERE]
// for obj := range h.minedBlockSub.Chan() {
// if ev, ok := obj.Data.(core.NewMinedBlockEvent); ok {
// h.BroadcastBlock(ev.Block, true) // First propagate block to peers
// h.BroadcastBlock(ev.Block, false) // Only then announce to the rest
// }
// }
}

}

// txBroadcastLoop announces new transactions to connected peers.
Expand Down
Loading