From 19741a98e0e43273532267a82ce812d9e08d1def Mon Sep 17 00:00:00 2001 From: Shrenuj Bansal Date: Mon, 11 Mar 2024 17:34:54 -0400 Subject: [PATCH] update proto comment and fix tests Signed-off-by: Shrenuj Bansal --- proto/dydxprotocol/subaccounts/subaccount.proto | 4 ++-- .../x/clob/types/message_cancel_order_test.go | 2 +- protocol/x/clob/types/message_place_order_test.go | 2 +- .../sending/types/message_create_transfer_test.go | 4 ++-- protocol/x/subaccounts/module_test.go | 7 +++++-- protocol/x/subaccounts/types/errors.go | 15 +++++++++++---- protocol/x/subaccounts/types/genesis_test.go | 7 ++++--- protocol/x/subaccounts/types/subaccount.pb.go | 4 ++-- 8 files changed, 28 insertions(+), 17 deletions(-) diff --git a/proto/dydxprotocol/subaccounts/subaccount.proto b/proto/dydxprotocol/subaccounts/subaccount.proto index 67dd5c017c..d0158a6375 100644 --- a/proto/dydxprotocol/subaccounts/subaccount.proto +++ b/proto/dydxprotocol/subaccounts/subaccount.proto @@ -11,8 +11,8 @@ option go_package = "github.com/dydxprotocol/v4-chain/protocol/x/subaccounts/typ message SubaccountId { // The address of the wallet that owns this subaccount. string owner = 1 [ (cosmos_proto.scalar) = "cosmos.AddressString" ]; - // < 128 Since 128 should be enough to start and it fits within - // 1 Byte (1 Bit needed to indicate that the first byte is the last). + // The unique number of this subaccount for the owner. + // Currently limited to 128*1000 subaccounts per owner. uint32 number = 2; } diff --git a/protocol/x/clob/types/message_cancel_order_test.go b/protocol/x/clob/types/message_cancel_order_test.go index d6b2f0681a..26d152c455 100644 --- a/protocol/x/clob/types/message_cancel_order_test.go +++ b/protocol/x/clob/types/message_cancel_order_test.go @@ -29,7 +29,7 @@ func TestMsgCancelOrder_ValidateBasic(t *testing.T) { OrderId: OrderId{ SubaccountId: satypes.SubaccountId{ Owner: sample.AccAddress(), - Number: uint32(9999), + Number: uint32(999_999), }, }, }, diff --git a/protocol/x/clob/types/message_place_order_test.go b/protocol/x/clob/types/message_place_order_test.go index 0b8cea0da8..ebd0db48a4 100644 --- a/protocol/x/clob/types/message_place_order_test.go +++ b/protocol/x/clob/types/message_place_order_test.go @@ -32,7 +32,7 @@ func TestMsgPlaceOrder_ValidateBasic(t *testing.T) { OrderId: OrderId{ SubaccountId: satypes.SubaccountId{ Owner: sample.AccAddress(), - Number: uint32(9999), + Number: uint32(999_999), }, }, }, diff --git a/protocol/x/sending/types/message_create_transfer_test.go b/protocol/x/sending/types/message_create_transfer_test.go index 302a90b3b9..19dfcf93ee 100644 --- a/protocol/x/sending/types/message_create_transfer_test.go +++ b/protocol/x/sending/types/message_create_transfer_test.go @@ -49,7 +49,7 @@ func TestMsgCreateTransfer_ValidateBasic(t *testing.T) { Transfer: &types.Transfer{ Sender: satypes.SubaccountId{ Owner: sample.AccAddress(), - Number: uint32(9999), + Number: uint32(999_999), }, Recipient: constants.Carl_Num0, }, @@ -63,7 +63,7 @@ func TestMsgCreateTransfer_ValidateBasic(t *testing.T) { Sender: constants.Carl_Num0, Recipient: satypes.SubaccountId{ Owner: sample.AccAddress(), - Number: uint32(9999), + Number: uint32(999_999), }, }, }, diff --git a/protocol/x/subaccounts/module_test.go b/protocol/x/subaccounts/module_test.go index 6ac07d6aff..df5a16221b 100644 --- a/protocol/x/subaccounts/module_test.go +++ b/protocol/x/subaccounts/module_test.go @@ -9,6 +9,9 @@ import ( "reflect" "testing" + "github.com/dydxprotocol/v4-chain/protocol/lib" + "github.com/dydxprotocol/v4-chain/protocol/x/subaccounts/types" + "github.com/dydxprotocol/v4-chain/protocol/app/module" "github.com/cosmos/cosmos-sdk/client" @@ -132,11 +135,11 @@ func TestAppModuleBasic_ValidateGenesisErrBadState_Number(t *testing.T) { cdc := codec.NewProtoCodec(module.InterfaceRegistry) - msg := fmt.Sprintf(`{"subaccounts": [{ "id": {"owner": "%s", "number": 128 } }]}`, sample.AccAddress()) + msg := fmt.Sprintf(`{"subaccounts": [{ "id": {"owner": "%s", "number": 128001 } }]}`, sample.AccAddress()) h := json.RawMessage(msg) err := am.ValidateGenesis(cdc, nil, h) - require.EqualError(t, err, "subaccount id number cannot exceed 127") + require.EqualError(t, err, "subaccount id number cannot exceed "+lib.IntToString(types.MaxSubaccountIdNumber)) } func TestAppModuleBasic_ValidateGenesis(t *testing.T) { diff --git a/protocol/x/subaccounts/types/errors.go b/protocol/x/subaccounts/types/errors.go index 12af34a0a7..91b4da30d2 100644 --- a/protocol/x/subaccounts/types/errors.go +++ b/protocol/x/subaccounts/types/errors.go @@ -2,7 +2,10 @@ package types // DONTCOVER -import errorsmod "cosmossdk.io/errors" +import ( + errorsmod "cosmossdk.io/errors" + "github.com/dydxprotocol/v4-chain/protocol/lib" +) // x/subaccounts module sentinel errors var ( @@ -18,9 +21,13 @@ var ( ErrProductPositionNotUpdatable = errorsmod.Register(ModuleName, 103, "product position is not updatable") // 200 - 299: subaccount id related. - ErrInvalidSubaccountIdNumber = errorsmod.Register(ModuleName, 200, "subaccount id number cannot exceed 127") - ErrInvalidSubaccountIdOwner = errorsmod.Register(ModuleName, 201, "subaccount id owner is an invalid address") - ErrDuplicateSubaccountIds = errorsmod.Register(ModuleName, 202, "duplicate subaccount id found in genesis") + ErrInvalidSubaccountIdNumber = errorsmod.Register( + ModuleName, + 200, + "subaccount id number cannot exceed "+lib.IntToString(MaxSubaccountIdNumber), + ) + ErrInvalidSubaccountIdOwner = errorsmod.Register(ModuleName, 201, "subaccount id owner is an invalid address") + ErrDuplicateSubaccountIds = errorsmod.Register(ModuleName, 202, "duplicate subaccount id found in genesis") // 300 - 399: asset position related. ErrAssetPositionsOutOfOrder = errorsmod.Register(ModuleName, 300, "asset positions are out of order") diff --git a/protocol/x/subaccounts/types/genesis_test.go b/protocol/x/subaccounts/types/genesis_test.go index 749b334ebb..5361b8ed4d 100644 --- a/protocol/x/subaccounts/types/genesis_test.go +++ b/protocol/x/subaccounts/types/genesis_test.go @@ -1,9 +1,10 @@ package types_test import ( - errorsmod "cosmossdk.io/errors" "testing" + errorsmod "cosmossdk.io/errors" + "github.com/dydxprotocol/v4-chain/protocol/dtypes" "github.com/dydxprotocol/v4-chain/protocol/testutil/sample" "github.com/dydxprotocol/v4-chain/protocol/x/subaccounts/types" @@ -85,13 +86,13 @@ func TestGenesisState_Validate(t *testing.T) { }, expectedError: types.ErrInvalidSubaccountIdOwner, }, - "invalid: id number is greater than 127": { + "invalid: id number is greater than 128_000": { genState: &types.GenesisState{ Subaccounts: []types.Subaccount{ { Id: &types.SubaccountId{ Owner: sample.AccAddress(), - Number: uint32(128), + Number: uint32(128_001), }, }, }, diff --git a/protocol/x/subaccounts/types/subaccount.pb.go b/protocol/x/subaccounts/types/subaccount.pb.go index 78a834d2fb..6ccef8bed5 100644 --- a/protocol/x/subaccounts/types/subaccount.pb.go +++ b/protocol/x/subaccounts/types/subaccount.pb.go @@ -27,8 +27,8 @@ const _ = proto.GoGoProtoPackageIsVersion3 // please upgrade the proto package type SubaccountId struct { // The address of the wallet that owns this subaccount. Owner string `protobuf:"bytes,1,opt,name=owner,proto3" json:"owner,omitempty"` - // < 128 Since 128 should be enough to start and it fits within - // 1 Byte (1 Bit needed to indicate that the first byte is the last). + // The unique number of this subaccount for the owner. + // Currently limited to 128*1000 subaccounts per owner. Number uint32 `protobuf:"varint,2,opt,name=number,proto3" json:"number,omitempty"` }