Optimized gas usage when writing objects

[Ronnieraj37]

Why This Should Be Merged

As discussed in issue #613 , I experimented with various methods for allocating objects and thoroughly tested each to determine the most efficient approach for gas optimization.

How This Works

When assigning mappings within an object, I found that individually assigning each item in the object was more efficient than assigning the entire object at once. However, for objects without mappings, a complete assignment was generally more efficient.

How This Was Tested

I ran all tests and observed the following gas usage reports:

Function Name	Initial Gas	New Gas	Gas Change
testCumulativeChurnRegistrationAndEndValidation()	744,442	743,364	1,078
testForceInitializeEndDelegation()	585,201	584,348	853
testForceInitializeEndDelegationInsufficientUptime()	532,187	531,334	853
testForceInitializeEndValidation()	489,396	488,856	540
testForceInitializeEndValidationInsufficientUptime()	475,388	474,849	539
testInitialWeightsTooLow()	5,509,390	5,446,569	62,821

View Complete Report

Summary of Gas Changes for These Tests

• Minimum reduction: 357
• Maximum reduction: 62,824
• Average reduction: 10,255.125

Documentation

For each object assigned more efficiently, I have included comments marked as:
// Efficient Assignment

Additional Modifications

The object PoSValidatorManagerStorage was restructured by separating certain fields into a new PoSValidatorManagerConfig object, as this improved assignment efficiency.

struct PoSValidatorManagerConfig {
    uint256 _minimumStakeAmount;
    uint256 _maximumStakeAmount;
    uint64 _minimumStakeDuration;
    uint16 _minimumDelegationFeeBips;
    uint64 _maximumStakeMultiplier;
    uint256 _weightToValueFactor;
    IRewardCalculator _rewardCalculator;
}

struct PoSValidatorManagerStorage {
    PoSValidatorManagerConfig config; // Created separate config object here
    mapping(bytes32 validationID => PoSValidatorInfo) _posValidatorInfo;
    mapping(bytes32 delegationID => Delegator) _delegatorStakes;
    mapping(bytes32 delegationID => uint256) _redeemableDelegatorRewards;
    mapping(bytes32 validationID => uint256) _redeemableValidatorRewards;
}

While this adjustment affects readability, I can remove it if preferred. Additionally, a similar implementation can be applied to ValidatorManagerStorage, but I have left it unchanged for now to confirm if this approach should be universally adopted.

struct ValidatorManagerStorage {
    bytes32 _subnetID;
    uint64 _churnPeriodSeconds;
    uint8 _maximumChurnPercentage;
    ValidatorChurnPeriod _churnTracker;
    mapping(bytes32 => bytes) _pendingRegisterValidationMessages;
    mapping(bytes32 => Validator) _validationPeriods;
    mapping(bytes => bytes32) _registeredValidators;
    bool _initializedValidatorSet;
}

[Ronnieraj37]

I've updated the ABI bindings and also run the linter. Please check if everything passes !

[cam-schultz]

Thanks for making these changes. I've left a handful of comments.

Also, it looks like this PR was automatically closed when ava-labs:validator-manager was deleted. Can you please reopen this PR against main?

[cam-schultz]

I don't think the gas savings here are worth the added code complexity and readability. Let's stick with keeping PoSValidatorManagerStorage as is.

[cam-schultz]

I don't think these comments are necessary, unless this is going against some established Solidity convention that would be unexpected to see.

[Ronnieraj37]

Thanks @cam-schultz I've created a new pr #633 with the new changes