Skip to content

Validator nft geographic data#93

Open
grantkee wants to merge 13 commits intomasterfrom
validator-nft-geographic-data
Open

Validator nft geographic data#93
grantkee wants to merge 13 commits intomasterfrom
validator-nft-geographic-data

Conversation

@grantkee
Copy link
Copy Markdown
Contributor

@grantkee grantkee commented Apr 3, 2026

Core feature

  • Added uint8 region (GSMA region identifier) to ValidatorInfo struct, packed into the existing storage slot
  • Added setValidatorRegion(address, uint8) governance function with onlyOwner + ConsensusNFT ownership check
  • Added ValidatorRegionUpdated(address indexed, uint8) event
  • Region defaults to 0 (unspecified) on stake; full uint8 range (0-255) accepted with no max constraint

isDelegated refactor

  • Removed bool isDelegated from ValidatorInfo struct to prevent storage slot overflow when adding region
  • Added isDelegated(address) view function that checks delegations[addr].delegator != address(0)
  • Removed genesis-time delegation initialization from _processGenesis (delegation records are set during delegateStake, not genesis)

Events

  • Added indexed to RewardsClaimed(address indexed claimant, uint256 rewards)
  • Added indexed to ValidatorRegionUpdated(address indexed validatorAddress, uint8 region)

Removed: EpochGasTarget

  • Deleted EpochGasTarget.sol, IEpochGasTarget.sol, and EpochGasTargetTest.t.sol
  • Removed from Deployments.sol and deployments.json
  • Removed associated artifacts

Documentation

  • Added Geographic Diversity section to design.md covering region mapping, setValidatorRegion, and shuffle algorithm
  • Added region invariants to invariants.md
  • Noted that the region-aware shuffle lives in the Rust protocol client, not in Solidity

Tests

  • Unit tests for setValidatorRegion across all validator statuses (Active, Staked, PendingActivation, PendingExit, Exited)
  • Tests for edge cases: max uint8 (255), reset to 0, no-NFT revert, non-owner revert
  • Fuzz test covering full uint8 range
  • Updated all ValidatorInfo constructors in tests to reflect struct change (isDelegated -> stakeVersion, region)

@grantkee grantkee self-assigned this Apr 3, 2026
@grantkee grantkee requested a review from chasebrownn April 8, 2026 00:06
@grantkee grantkee marked this pull request as ready for review April 8, 2026 00:06
Copy link
Copy Markdown
Collaborator

@chasebrownn chasebrownn left a comment

Choose a reason for hiding this comment

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

Just a few minor thoughts - nothing crazy.

Comment on lines +245 to +247
function isDelegated(address validatorAddress) external view returns (bool) {
return _isDelegated(validatorAddress);
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Do we want this to return true if a validator retires? Seems this state does not change if there is an unstake or burn of the validator.

Comment on lines +136 to +140
function setValidatorRegion(address validatorAddress, uint8 region) external onlyOwner {
_checkConsensusNFTOwner(validatorAddress);
validators[validatorAddress].region = region;
emit ValidatorRegionUpdated(validatorAddress, region);
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Should we sanitize region for invalid regions? I know we're only supporting 0-8 currently, but that leaves 9-255 as possible invalid inputs that could throw things off.

false,
isDelegated,
stakeVersion
blsPubkey, validatorAddress, PENDING_EPOCH, uint32(0), ValidatorStatus.Staked, false, stakeVersion, uint8(0)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Do we not want to copy over the existing region value to the new validator?

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.

2 participants