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.

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

feat(avs):implement whitelist feature for AVS #262

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
+290 −138
Open

feat(avs):implement whitelist feature for AVS #262

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7e89c4a
implement whitelist feature for AVS
trestinlsd Dec 6, 2024
00eeed8
fix lint err
trestinlsd Dec 11, 2024
70cd42e
fix some comments
trestinlsd Dec 12, 2024
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
4 changes: 4 additions & 0 deletions precompiles/avs/IAVSManager.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ interface IAVSManager {
/// @param slashAddr The slash address of AVS.
Copy link
Contributor

Choose a reason for hiding this comment

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

Have you verified that this file compiles? Too many local variables (within a function) can cause Solidity to complain "stack too deep"

/// @param rewardAddr The reward address of AVS.
/// @param avsOwnerAddress The owners who have permission for AVS.
/// @param whitelistAddress The whitelist address of the operator.
/// @param assetIds The basic asset information of AVS.
/// @param avsUnbondingPeriod The unbonding duration of AVS.
/// @param minSelfDelegation The minimum delegation amount for an operator.
Expand All @@ -65,6 +66,7 @@ interface IAVSManager {
address slashAddr,
address rewardAddr,
string[] memory avsOwnerAddress,
string[] memory whitelistAddress,
trestinlsd marked this conversation as resolved.
Show resolved Hide resolved
string[] memory assetIds,
uint64 avsUnbondingPeriod,
uint64 minSelfDelegation,
Expand All @@ -80,6 +82,7 @@ interface IAVSManager {
/// @param slashAddr The slash address of AVS.
/// @param rewardAddr The reward address of AVS.
/// @param avsOwnerAddress The owners who have permission for AVS.
/// @param whitelistAddress The whitelist address of the operator.
/// @param assetIds The basic asset information of AVS.
/// @param avsUnbondingPeriod The unbonding duration of AVS.
/// @param minSelfDelegation The minimum delegation amount for an operator.
Expand All @@ -96,6 +99,7 @@ interface IAVSManager {
address slashAddr,
address rewardAddr,
string[] memory avsOwnerAddress,
string[] memory whitelistAddress,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why choose string type for these addresses and not address which is Solidity native and has great support? Each exo1... bech32-encoded address can be represented as a hex address and vice-versa.

string[] memory assetIds,
uint64 avsUnbondingPeriod,
uint64 minSelfDelegation,
Expand Down
10 changes: 10 additions & 0 deletions precompiles/avs/abi.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -646,6 +646,11 @@
"name": "avsOwnerAddress",
"type": "string[]"
},
{
"internalType": "string[]",
"name": "whitelistAddress",
"type": "string[]"
},
{
"internalType": "string[]",
"name": "assetIds",
Expand Down Expand Up @@ -778,6 +783,11 @@
"name": "avsOwnerAddress",
"type": "string[]"
},
{
"internalType": "string[]",
"name": "whitelistAddress",
"type": "string[]"
},
{
"internalType": "string[]",
"name": "assetIds",
Expand Down
2 changes: 2 additions & 0 deletions precompiles/avs/avs_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ func (suite *AVSManagerPrecompileSuite) TestRegisterAVS() {
common.HexToAddress(slashAddress),
common.HexToAddress(rewardAddress),
avsOwnerAddress,
avsOwnerAddress,
trestinlsd marked this conversation as resolved.
Show resolved Hide resolved
assetID,
avsUnbondingPeriod,
minSelfDelegation,
Expand Down Expand Up @@ -364,6 +365,7 @@ func (suite *AVSManagerPrecompileSuite) TestUpdateAVS() {
common.HexToAddress(slashAddress),
common.HexToAddress(rewardAddress),
avsOwnerAddress,
avsOwnerAddress,
assetID,
avsUnbondingPeriod,
minSelfDelegation,
Expand Down
72 changes: 49 additions & 23 deletions precompiles/avs/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,38 +68,51 @@ func (p Precompile) GetAVSParamsFromInputs(_ sdk.Context, args []interface{}) (*
exoAddresses[i] = accAddr.String()
}
avsParams.AvsOwnerAddress = exoAddresses

// bech32
whitelistAddress, ok := args[7].([]string)
if !ok || whitelistAddress == nil {
trestinlsd marked this conversation as resolved.
Show resolved Hide resolved
return nil, fmt.Errorf(exocmn.ErrContractInputParaOrType, 7, "[]string", whitelistAddress)
}
exoWhiteAddresses := make([]string, len(whitelistAddress))
for i, addr := range whitelistAddress {
accAddr, err := sdk.AccAddressFromBech32(addr)
if err != nil {
return nil, fmt.Errorf(exocmn.ErrContractInputParaOrType, 7, "[]string", whitelistAddress)
}
exoAddresses[i] = accAddr.String()
trestinlsd marked this conversation as resolved.
Show resolved Hide resolved
}
avsParams.WhitelistAddress = exoWhiteAddresses
// string, since it is the address_id representation
assetID, ok := args[7].([]string)
assetID, ok := args[8].([]string)
if !ok || len(assetID) == 0 {
return nil, fmt.Errorf(exocmn.ErrContractInputParaOrType, 7, "[]string", assetID)
return nil, fmt.Errorf(exocmn.ErrContractInputParaOrType, 8, "[]string", assetID)
}
avsParams.AssetID = assetID

unbondingPeriod, ok := args[8].(uint64)
unbondingPeriod, ok := args[9].(uint64)
if !ok || unbondingPeriod == 0 {
return nil, fmt.Errorf(exocmn.ErrContractInputParaOrType, 8, "uint64", unbondingPeriod)
return nil, fmt.Errorf(exocmn.ErrContractInputParaOrType, 9, "uint64", unbondingPeriod)
}
avsParams.UnbondingPeriod = unbondingPeriod

minSelfDelegation, ok := args[9].(uint64)
minSelfDelegation, ok := args[10].(uint64)
if !ok {
return nil, fmt.Errorf(exocmn.ErrContractInputParaOrType, 9, "uint64", minSelfDelegation)
return nil, fmt.Errorf(exocmn.ErrContractInputParaOrType, 10, "uint64", minSelfDelegation)
}
avsParams.MinSelfDelegation = minSelfDelegation

epochIdentifier, ok := args[10].(string)
epochIdentifier, ok := args[11].(string)
if !ok || epochIdentifier == "" {
return nil, fmt.Errorf(exocmn.ErrContractInputParaOrType, 10, "string", epochIdentifier)
return nil, fmt.Errorf(exocmn.ErrContractInputParaOrType, 11, "string", epochIdentifier)
}
avsParams.EpochIdentifier = epochIdentifier

// The parameters below are used when creating tasks, to ensure that the minimum criteria are met by the set
// of operators.

taskParam, ok := args[11].([]uint64)
taskParam, ok := args[12].([]uint64)
if !ok || taskParam == nil || len(taskParam) != 4 {
return nil, fmt.Errorf(exocmn.ErrContractInputParaOrType, 11, "[]string", taskParam)
return nil, fmt.Errorf(exocmn.ErrContractInputParaOrType, 12, "[]string", taskParam)
}
minOptInOperators := taskParam[0]
avsParams.MinOptInOperators = minOptInOperators
Expand Down Expand Up @@ -166,43 +179,56 @@ func (p Precompile) GetAVSParamsFromUpdateInputs(_ sdk.Context, args []interface
for i, addr := range avsOwnerAddress {
accAddr, err := sdk.AccAddressFromBech32(addr)
if err != nil {
return nil, fmt.Errorf(exocmn.ErrContractInputParaOrType, 7, "[]string", avsOwnerAddress)
return nil, fmt.Errorf(exocmn.ErrContractInputParaOrType, 6, "[]string", avsOwnerAddress)
}
exoAddresses[i] = accAddr.String()
}
avsParams.AvsOwnerAddress = exoAddresses

// bech32
whitelistAddress, ok := args[7].([]string)
trestinlsd marked this conversation as resolved.
Show resolved Hide resolved
if !ok || whitelistAddress == nil {
return nil, fmt.Errorf(exocmn.ErrContractInputParaOrType, 7, "[]string", whitelistAddress)
}
exoWhiteAddresses := make([]string, len(whitelistAddress))
for i, addr := range whitelistAddress {
accAddr, err := sdk.AccAddressFromBech32(addr)
if err != nil {
return nil, fmt.Errorf(exocmn.ErrContractInputParaOrType, 7, "[]string", whitelistAddress)
}
exoAddresses[i] = accAddr.String()
}
avsParams.WhitelistAddress = exoWhiteAddresses
// string, since it is the address_id representation
assetID, ok := args[7].([]string)
assetID, ok := args[8].([]string)
if !ok {
return nil, fmt.Errorf(exocmn.ErrContractInputParaOrType, 7, "[]string", assetID)
return nil, fmt.Errorf(exocmn.ErrContractInputParaOrType, 8, "[]string", assetID)
}
avsParams.AssetID = assetID

unbondingPeriod, ok := args[8].(uint64)
unbondingPeriod, ok := args[9].(uint64)
if !ok {
return nil, fmt.Errorf(exocmn.ErrContractInputParaOrType, 8, "uint64", unbondingPeriod)
return nil, fmt.Errorf(exocmn.ErrContractInputParaOrType, 9, "uint64", unbondingPeriod)
}
avsParams.UnbondingPeriod = unbondingPeriod

minSelfDelegation, ok := args[9].(uint64)
minSelfDelegation, ok := args[10].(uint64)
if !ok {
return nil, fmt.Errorf(exocmn.ErrContractInputParaOrType, 9, "uint64", minSelfDelegation)
return nil, fmt.Errorf(exocmn.ErrContractInputParaOrType, 10, "uint64", minSelfDelegation)
}
avsParams.MinSelfDelegation = minSelfDelegation

epochIdentifier, ok := args[10].(string)
epochIdentifier, ok := args[11].(string)
if !ok {
return nil, fmt.Errorf(exocmn.ErrContractInputParaOrType, 10, "string", epochIdentifier)
return nil, fmt.Errorf(exocmn.ErrContractInputParaOrType, 11, "string", epochIdentifier)
}
avsParams.EpochIdentifier = epochIdentifier

// The parameters below are used when creating tasks, to ensure that the minimum criteria are met by the set
// of operators.

taskParam, ok := args[11].([]uint64)
taskParam, ok := args[12].([]uint64)
if !ok || taskParam == nil || len(taskParam) != 4 {
return nil, fmt.Errorf(exocmn.ErrContractInputParaOrType, 11, "[]string", taskParam)
return nil, fmt.Errorf(exocmn.ErrContractInputParaOrType, 12, "[]string", taskParam)
trestinlsd marked this conversation as resolved.
Show resolved Hide resolved
}
minOptInOperators := taskParam[0]
avsParams.MinOptInOperators = minOptInOperators
Expand Down
3 changes: 2 additions & 1 deletion proto/exocore/avs/v1/tx.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,10 @@ message AVSInfo {
(gogoproto.customtype) = "github.com/cosmos/cosmos-sdk/types.Dec",
(gogoproto.nullable) = false
];

// asset_reward_commission_epoch_basis is the avs reward distribution based on asset per eopch end.
map<string, int64> asset_reward_amount_epoch_basis = 18;
// whitelist_address are the bech32 addresses ,whitelist address of supported operators
repeated string whitelist_address = 19;
}

// Status and proof of each operator
Expand Down
21 changes: 21 additions & 0 deletions x/avs/keeper/avs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package keeper
import (
"fmt"
"math/big"
"slices"
"strconv"
"strings"

Expand Down Expand Up @@ -224,3 +225,23 @@ func (k *Keeper) GetAllChainIDInfos(ctx sdk.Context) ([]types.ChainIDInfo, error
}
return ret, nil
}

// IsWhitelisted check if operator is in the whitelist
func (k *Keeper) IsWhitelisted(ctx sdk.Context, avsAddr, operatorAddr string) (bool, error) {
avsInfo, err := k.GetAVSInfo(ctx, avsAddr)
if err != nil {
return false, errorsmod.Wrap(err, fmt.Sprintf("IsWhitelisted: key is %s", avsAddr))
}
_, err = sdk.AccAddressFromBech32(operatorAddr)
if err != nil {
return false, errorsmod.Wrap(err, "IsWhitelisted: error occurred when parse acc address from Bech32")
}
// Currently avs has no whitelist set and any operator can optin
if len(avsInfo.Info.WhitelistAddress) == 0 {
return true, nil
}
if !slices.Contains(avsInfo.Info.WhitelistAddress, operatorAddr) {
return false, errorsmod.Wrap(err, "not in the whitelist address of supported operators")
}
return true, nil
}
trestinlsd marked this conversation as resolved.
Show resolved Hide resolved
6 changes: 6 additions & 0 deletions x/avs/keeper/avs_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ func (suite *AVSTestSuite) TestAVS() {
avsAddress := suite.avsAddress
avsOwnerAddress := []string{"exo13h6xg79g82e2g2vhjwg7j4r2z2hlncelwutkjr", "exo13h6xg79g82e2g2vhjwg7j4r2z2hlncelwutkj1", "exo13h6xg79g82e2g2vhjwg7j4r2z2hlncelwutkj2"}
assetID := suite.AssetIDs
operatorAddress := sdk.AccAddress(utiltx.GenerateAddress().Bytes()).String()

avs := &types.AVSInfo{
Name: avsName,
AvsAddress: avsAddress.String(),
Expand All @@ -38,11 +40,15 @@ func (suite *AVSTestSuite) TestAVS() {
AvsSlash: sdk.MustNewDecFromStr("0.001"),
AvsReward: sdk.MustNewDecFromStr("0.002"),
TaskAddr: "exo13h6xg79g82e2g2vhjwg7j4r2z2hlncelwutkjr",
WhitelistAddress: []string{operatorAddress},
}

err := suite.App.AVSManagerKeeper.SetAVSInfo(suite.Ctx, avs)
suite.NoError(err)

whitelisted, err := suite.App.AVSManagerKeeper.IsWhitelisted(suite.Ctx, avsAddress.String(), operatorAddress)
suite.NoError(err)
suite.Equal(whitelisted, true)
trestinlsd marked this conversation as resolved.
Show resolved Hide resolved
info, err := suite.App.AVSManagerKeeper.GetAVSInfo(suite.Ctx, avsAddress.String())

suite.NoError(err)
Expand Down
6 changes: 5 additions & 1 deletion x/avs/keeper/keeper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,8 @@ func (k Keeper) UpdateAVSInfo(ctx sdk.Context, params *types.AVSRegisterOrDeregi
// #nosec G115
AvsSlash: sdk.NewDecWithPrec(int64(params.AvsSlash), 2),
// #nosec G115
AvsReward: sdk.NewDecWithPrec(int64(params.AvsReward), 2),
AvsReward: sdk.NewDecWithPrec(int64(params.AvsReward), 2),
WhitelistAddress: params.WhitelistAddress,
trestinlsd marked this conversation as resolved.
Show resolved Hide resolved
}

return k.SetAVSInfo(ctx, avs)
Expand Down Expand Up @@ -185,6 +186,9 @@ func (k Keeper) UpdateAVSInfo(ctx sdk.Context, params *types.AVSRegisterOrDeregi
if params.AvsOwnerAddress != nil {
avs.AvsOwnerAddress = params.AvsOwnerAddress
}
if params.WhitelistAddress != nil {
avs.WhitelistAddress = params.WhitelistAddress
}
Comment on lines +189 to +191
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add validation for WhitelistAddress during AVS updates

Similar to the registration flow, the update flow also lacks address validation for WhitelistAddress.

Add validation before updating WhitelistAddress:

 if params.WhitelistAddress != nil {
+    if err := k.validateAddresses(params.WhitelistAddress); err != nil {
+        return err
+    }
     avs.WhitelistAddress = params.WhitelistAddress
 }
📝 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
if params.WhitelistAddress != nil {
avs.WhitelistAddress = params.WhitelistAddress
}
if params.WhitelistAddress != nil {
if err := k.validateAddresses(params.WhitelistAddress); err != nil {
return err
}
avs.WhitelistAddress = params.WhitelistAddress
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Please consider this comment from the AI.

Copy link
Contributor

Choose a reason for hiding this comment

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

@MaxMustermann2, here's an example of how you could incorporate address validation for the WhitelistAddress before updating:

// helper function to validate a list of Bech32 addresses
func (k Keeper) validateAddresses(addresses []string) error {
	for _, addr := range addresses {
		if _, err := sdk.AccAddressFromBech32(addr); err != nil {
			return errorsmod.Wrap(types.ErrInvalidAddr, fmt.Sprintf("Invalid whitelist address: %s", addr))
		}
	}
	return nil
}

// snippet inside UpdateAVSInfo
if params.WhitelistAddress != nil {
	if err := k.validateAddresses(params.WhitelistAddress); err != nil {
		return err
	}
	avs.WhitelistAddress = params.WhitelistAddress
}

This ensures each address is checked for validity prior to updating the whitelist.

if params.AssetID != nil {
avs.AssetIDs = params.AssetID
if err := k.ValidateAssetIDs(ctx, params.AssetID); err != nil {
Expand Down
Loading
Loading