Skip to content

Feature: StakeHub Contract Interface Implementation#3045

Merged
zzzckck merged 13 commits intodevelopfrom
feature/node-id-registration
Apr 28, 2025
Merged

Feature: StakeHub Contract Interface Implementation#3045
zzzckck merged 13 commits intodevelopfrom
feature/node-id-registration

Conversation

@MatusKysel
Copy link
Contributor

Add: StakeHub Contract Interface Implementation

Changes

  • Created new file consensus/parlia/stakehub.go to implement the StakeHub contract interface
  • Implemented functions for validator and node ID management:
    • GetValidators: Retrieves validators from the StakeHubContract
    • ListNodeIDsFor: Retrieves node IDs for given validators
    • GetNodeIDs: Returns flattened array of all node IDs for current validators
    • AddNodeIDs: Creates signed transaction to add node IDs to the StakeHub contract

Technical Details

  • Implemented contract interaction using ethAPI.Call
  • Added proper error handling and type safety
  • Used enode.ID for node identification
  • Included transaction signing functionality

Why

This implementation provides a clean interface for interacting with the StakeHub contract, handling validator management and node ID registration in a type-safe and maintainable way.

@MatusKysel MatusKysel requested review from Copilot and galaio April 23, 2025 20:25
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements the StakeHub contract interface to manage validator and node ID registrations. The key changes include:

  • Adding node ID registration logic during mining startup in eth/backend.go.
  • Implementing StakeHub contract functions (GetValidators, ListNodeIDsFor, GetNodeIDs, AddNodeIDs) in consensus/parlia/stakehub.go.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
eth/backend.go Adds logic to check and register the node ID by invoking parlia.GetNodeIDs and AddNodeIDs.
consensus/parlia/stakehub.go Introduces the StakeHub contract interface functions for validator and node ID management.

@MatusKysel MatusKysel marked this pull request as ready for review April 24, 2025 08:29
@galaio galaio changed the base branch from develop_evn to develop April 25, 2025 09:25
@MatusKysel MatusKysel force-pushed the feature/node-id-registration branch from 04b6082 to da8bdb9 Compare April 25, 2025 09:32
@MatusKysel MatusKysel force-pushed the feature/node-id-registration branch from a8a1314 to 259fc7c Compare April 25, 2025 11:15
@galaio galaio force-pushed the feature/node-id-registration branch from 2e1fd70 to 5f6f7fc Compare April 27, 2025 14:10
Copy link

@Jojad9 Jojad9 left a comment

Choose a reason for hiding this comment

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

Feature/node-id - registration

@MatusKysel MatusKysel force-pushed the feature/node-id-registration branch from ed5efa0 to e651fe4 Compare April 28, 2025 07:00
zzzckck

This comment was marked as off-topic.

@MatusKysel MatusKysel requested a review from Copilot April 28, 2025 08:10
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@MatusKysel MatusKysel requested a review from Copilot April 28, 2025 08:11
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

zzzckck
zzzckck previously approved these changes Apr 28, 2025
Copy link
Collaborator

@zzzckck zzzckck left a comment

Choose a reason for hiding this comment

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

LGTM
the SystemContract bytecode matches: bnb-chain/bsc-genesis-contract@3e38d7b

@MatusKysel MatusKysel requested a review from Copilot April 28, 2025 08:31
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements the StakeHub contract interface and integrates node ID management into the consensus engine. It adds new functions for retrieving validators and node IDs, creates a signed transaction for adding node IDs, and updates configurations and upgrade paths to support the StakeHub contract.

  • Adds a new file (consensus/parlia/stakehub.go) with functions GetValidators, GetNodeIDs, AddNodeIDs, and GetNodeIDsMap.
  • Updates Ethereum configuration files (eth/ethconfig/gen_config.go and config.go) to include a new ValidatorNodeIDsToAdd field.
  • Modifies backend shutdown channel usage and integrates Maxwell upgrade configurations in system contracts.

Reviewed Changes

Copilot reviewed 7 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
eth/ethconfig/gen_config.go Adds ValidatorNodeIDsToAdd field for TOML marshalling and unmarshalling
eth/ethconfig/config.go Incorporates new ValidatorNodeIDsToAdd field into the configuration structure
eth/backend.go Replaces stopReportCh with stopCh and adds a goroutine for node ID registration
core/systemcontracts/upgrade.go Adds Maxwell upgrade configuration for the StakeHub contract
core/systemcontracts/maxwell/types.go Embeds contract code files for Maxwell upgrades
consensus/parlia/stakehub.go Implements the StakeHub contract interface functions and node ID management
consensus/parlia/abi.go Updates the ABI definition for the StakeHub contract interface with new function names
Files not reviewed (3)
  • core/systemcontracts/maxwell/chapel/StakeHubContract: Language not supported
  • core/systemcontracts/maxwell/mainnet/StakeHubContract: Language not supported
  • core/systemcontracts/maxwell/rialto/StakeHubContract: Language not supported
Comments suppressed due to low confidence (1)

consensus/parlia/abi.go:4117

  • Verify that the renaming and type adjustments in the ABI for the addNodeIDs function correctly match the deployed StakeHub contract interface.
"name": "addNodeIDs",

to := common.HexToAddress(systemcontracts.StakeHubContract)
hexData := hexutil.Bytes(data)
hexNonce := hexutil.Uint64(nonce)
gas, err := p.ethAPI.EstimateGas(context.Background(), ethapi.TransactionArgs{
Copy link

Copilot AI Apr 28, 2025

Choose a reason for hiding this comment

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

Consider providing a context with a timeout for the gas estimation call to avoid potential indefinite hangs.

Suggested change
gas, err := p.ethAPI.EstimateGas(context.Background(), ethapi.TransactionArgs{
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()
gas, err := p.ethAPI.EstimateGas(ctx, ethapi.TransactionArgs{

Copilot uses AI. Check for mistakes.

// GetValidators retrieves validators from the StakeHubContract
// It returns operator addresses, credit addresses, and total length of validators
func (p *Parlia) GetValidators(blockNumber uint64, offset, limit *big.Int) ([]common.Address, []common.Address, *big.Int, error) {

This comment was marked as resolved.

@NathanBSC
Copy link
Contributor

NathanBSC commented Apr 28, 2025

Initially, I thought that 1000 might be excessive and could lead to inefficiency, as the contract might return many validators that most of would never be used.
However, I later realized it doesn't matter much, since this function is not on the critical path.

@zzzckck zzzckck merged commit d924a81 into develop Apr 28, 2025
7 checks passed
@MatusKysel MatusKysel deleted the feature/node-id-registration branch April 28, 2025 11:39
galaio pushed a commit to galaio/bsc that referenced this pull request May 29, 2025
galaio pushed a commit to galaio/bsc that referenced this pull request May 29, 2025
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.

7 participants