Skip to content

Commit

Permalink
Fix/query sign requests broadcast type (#1127)
Browse files Browse the repository at this point in the history
* make BroadcastType optional (nullable) in QuerySignRequestsRequest.
Null value means "query sign request with all broadcast types"

* fix precompile

* regenerate IWarden abi

* update tests to query all broadcast types

* fix rabbit

* update changelog
  • Loading branch information
mn13 authored Dec 5, 2024
1 parent fae2674 commit d7f79f9
Show file tree
Hide file tree
Showing 10 changed files with 893 additions and 533 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
* (precompiles) #1077 Change status fields from int32 to enums/uint8 in precompile ABIs for improved type safety and consistency
* (wardend) Bump IAVL to v1.2.2. Fixes some potential apphash mismatches that happen in some rare cases.
* (x/async) Scaffold new module with create/read operations
* (x/warden) Sign requests query return all request (not only with broadcastType=BroadcastType.Disabled)

### Bug Fixes

Expand Down
908 changes: 525 additions & 383 deletions api/warden/warden/v1beta3/query.pulsar.go

Large diffs are not rendered by default.

25 changes: 13 additions & 12 deletions precompiles/warden/IWarden.go

Large diffs are not rendered by default.

10 changes: 8 additions & 2 deletions precompiles/warden/IWarden.sol
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,12 @@ enum BroadcastType {
Automatic
}

enum OptionalBroadcastType {
Nonspecified,
Disabled,
Automatic
}

/**
* @author Warden Team
* @title x/warden Interface
Expand Down Expand Up @@ -431,14 +437,14 @@ interface IWarden {
/// @param pageRequest The pagination details
/// @param keychainId The id of the keychain
/// @param status The sign requests status
/// @param broadcastType The broadcast type
/// @param optionalBroadcastType The optional broadcast type to query
/// @return signRequests An array of `SignRequest` structs containing the retrieved sign requests
/// @return pageResponse pagination details
function signRequests(
Types.PageRequest calldata pageRequest,
uint64 keychainId,
SignRequestStatus status,
BroadcastType broadcastType
OptionalBroadcastType optionalBroadcastType
) external view returns(SignRequest[] memory signRequests, Types.PageResponse memory pageResponse);

/// @dev Defines a method to query space by id.
Expand Down
4 changes: 2 additions & 2 deletions precompiles/warden/abi.json
Original file line number Diff line number Diff line change
Expand Up @@ -1926,8 +1926,8 @@
"type": "uint8"
},
{
"internalType": "enum BroadcastType",
"name": "broadcastType",
"internalType": "enum OptionalBroadcastType",
"name": "optionalBroadcastType",
"type": "uint8"
}
],
Expand Down
49 changes: 37 additions & 12 deletions precompiles/warden/query_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -395,23 +395,48 @@ func newSignRequestsRequest(method *abi.Method, args []interface{}) (*types.Quer
if _, ok := types.SignRequestStatus_name[int32(input.Status)]; !ok {
return nil, fmt.Errorf("invalid Status value: %d", input.Status)
}
if _, ok := types.BroadcastType_name[int32(input.BroadcastType)]; !ok {
return nil, fmt.Errorf("invalid BroadcastType value: %d", input.BroadcastType)

broadcastType := convertToBroadcastType(input.OptionalBroadcastType)
if broadcastType != nil {
if _, ok := types.BroadcastType_name[int32(*broadcastType)]; !ok {
return nil, fmt.Errorf("invalid BroadcastType value: %d", *broadcastType)
}

querySignRequestBroadcastType := types.QuerySignRequestsRequest_BroadcastType{
BroadcastType: *convertToBroadcastType(input.OptionalBroadcastType),
}
return &types.QuerySignRequestsRequest{
Pagination: &input.PageRequest,
KeychainId: input.KeychainId,
Status: types.SignRequestStatus(int32(input.Status)),
OptionalBroadcastType: &querySignRequestBroadcastType,
}, nil
} else {
return &types.QuerySignRequestsRequest{
Pagination: &input.PageRequest,
KeychainId: input.KeychainId,
Status: types.SignRequestStatus(int32(input.Status)),
OptionalBroadcastType: nil,
}, nil
}
}

return &types.QuerySignRequestsRequest{
Pagination: &input.PageRequest,
KeychainId: input.KeychainId,
Status: types.SignRequestStatus(int32(input.Status)),
BroadcastType: types.BroadcastType(int32(input.BroadcastType)),
}, nil
type optionalBroadcastType = uint8

func convertToBroadcastType(obt optionalBroadcastType) *types.BroadcastType {
if obt == 0 {
return nil
}

broadcastType := types.BroadcastType(int32(obt - 1))
return &broadcastType
}

type signRequestsInput struct {
PageRequest query.PageRequest
KeychainId uint64
Status uint8
BroadcastType uint8
PageRequest query.PageRequest
KeychainId uint64
Status uint8
OptionalBroadcastType optionalBroadcastType
}

type signRequestsOutput struct {
Expand Down
8 changes: 6 additions & 2 deletions proto/warden/warden/v1beta3/query.proto
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ package warden.warden.v1beta3;
import "amino/amino.proto";
import "gogoproto/gogo.proto";
import "google/api/annotations.proto";
import "google/protobuf/struct.proto";
import "cosmos/base/query/v1beta1/pagination.proto";
import "warden/warden/v1beta3/key.proto";
import "warden/warden/v1beta3/keychain.proto";
Expand Down Expand Up @@ -181,8 +182,11 @@ message AddressResponse {
message QuerySignRequestsRequest {
cosmos.base.query.v1beta1.PageRequest pagination = 1;
uint64 keychain_id = 2;
SignRequestStatus status = 3 [ (gogoproto.nullable) = true ];
BroadcastType broadcast_type = 4 [ (gogoproto.nullable) = true ];
SignRequestStatus status = 3 [(gogoproto.nullable) = true];
oneof optional_broadcast_type {
google.protobuf.NullValue null = 4;
BroadcastType broadcast_type = 5;
}
}

message QuerySignRequestsResponse {
Expand Down
107 changes: 87 additions & 20 deletions tests/cases/warden_precompile_action.go
Original file line number Diff line number Diff line change
Expand Up @@ -215,8 +215,8 @@ func (c *Test_WardenPrecompileAction) Run(t *testing.T, ctx context.Context, bui
require.NoError(t, err)
require.Len(t, keyRequests2.KeyRequests, 1)

// newSignRequest
newSignRequestTx, err := iWardenClient.NewSignRequest(
// newSignRequest automatic
signRequestAutomatic, err := iWardenClient.NewSignRequest(
alice.TransactOps(t, context.Background(), evmClient),
1,
[]byte{30, 134, 120, 103, 230, 84, 237, 151, 116, 242, 69, 17, 228, 215, 27, 180, 86, 107, 152, 98, 133, 215, 201, 146, 4, 157, 189, 118, 13, 42, 35, 142},
Expand All @@ -227,30 +227,30 @@ func (c *Test_WardenPrecompileAction) Run(t *testing.T, ctx context.Context, bui
0,
"any(1, warden.space.owners)",
"any(1, warden.space.owners)",
uint8(types.BroadcastType_BROADCAST_TYPE_DISABLED))
uint8(types.BroadcastType_BROADCAST_TYPE_AUTOMATIC))
require.NoError(t, err)

newSignRequestTxReceipt, err := bind.WaitMined(ctx, evmClient, newSignRequestTx)
newSignRequestAutomaticTxReceipt, err := bind.WaitMined(ctx, evmClient, signRequestAutomatic)
require.NoError(t, err)

newSignRequestEvents, err := checks.GetParsedEventsOnly(newSignRequestTxReceipt, iWardenClient.ParseNewSignRequest)
newSignRequestAutomaticEvents, err := checks.GetParsedEventsOnly(newSignRequestAutomaticTxReceipt, iWardenClient.ParseNewSignRequest)
require.NoError(t, err)

require.Len(t, newSignRequestEvents, 1)
require.Equal(t, newSignRequestEvents[0].Id, uint64(1))
require.Equal(t, newSignRequestEvents[0].KeyId, uint64(1))
require.Equal(t, newSignRequestEvents[0].Creator, alice.EthAddress(t))
require.Equal(t, newSignRequestEvents[0].BroadcastType, uint8(types.BroadcastType_BROADCAST_TYPE_DISABLED))
require.Len(t, newSignRequestAutomaticEvents, 1)
require.Equal(t, newSignRequestAutomaticEvents[0].Id, uint64(1))
require.Equal(t, newSignRequestAutomaticEvents[0].KeyId, uint64(1))
require.Equal(t, newSignRequestAutomaticEvents[0].Creator, alice.EthAddress(t))
require.Equal(t, newSignRequestAutomaticEvents[0].BroadcastType, uint8(types.BroadcastType_BROADCAST_TYPE_AUTOMATIC))

actions4, err := iActClient.Actions(alice.CallOps(t), act.TypesPageRequest{})

require.NoError(t, err)
require.Len(t, actions4.Actions, 4)

signRequests, err := iWardenClient.SignRequests(alice.CallOps(t), warden.TypesPageRequest{}, 1, uint8(types.SignRequestStatus_SIGN_REQUEST_STATUS_PENDING), 0)
signRequestsAutomatic, err := iWardenClient.SignRequests(alice.CallOps(t), warden.TypesPageRequest{}, 1, uint8(types.SignRequestStatus_SIGN_REQUEST_STATUS_PENDING), uint8(types.BroadcastType_BROADCAST_TYPE_AUTOMATIC)+1)

require.NoError(t, err)
require.Len(t, signRequests.SignRequests, 1)
require.Len(t, signRequestsAutomatic.SignRequests, 1)
require.Equal(t, warden.SignRequest{
Id: 1,
Creator: alice.EthAddress(t),
Expand All @@ -260,16 +260,83 @@ func (c *Test_WardenPrecompileAction) Run(t *testing.T, ctx context.Context, bui
DataForSigning: []byte{30, 134, 120, 103, 230, 84, 237, 151, 116, 242, 69, 17, 228, 215, 27, 180, 86, 107, 152, 98, 133, 215, 201, 146, 4, 157, 189, 118, 13, 42, 35, 142},
DeductedKeychainFees: []warden.TypesCoin{},
Result: []byte{},
BroadcastType: uint8(types.BroadcastType_BROADCAST_TYPE_AUTOMATIC),
}, signRequestsAutomatic.SignRequests[0])

signRequestsDisabled, err := iWardenClient.SignRequests(
alice.CallOps(t),
warden.TypesPageRequest{},
1,
uint8(types.SignRequestStatus_SIGN_REQUEST_STATUS_PENDING),
uint8(types.BroadcastType_BROADCAST_TYPE_DISABLED+1))
require.NoError(t, err)
require.Len(t, signRequestsDisabled.SignRequests, 0)

// newSignRequest disabled
newSignRequestDisabledTx, err := iWardenClient.NewSignRequest(
alice.TransactOps(t, context.Background(), evmClient),
1,
[]byte{30, 134, 120, 103, 230, 84, 237, 151, 116, 242, 69, 17, 228, 215, 27, 180, 86, 107, 152, 98, 133, 215, 201, 146, 4, 157, 189, 118, 13, 42, 35, 142},
[][]byte{},
[]byte{},
[]warden.TypesCoin{},
2,
0,
"any(1, warden.space.owners)",
"any(1, warden.space.owners)",
uint8(types.BroadcastType_BROADCAST_TYPE_DISABLED))
require.NoError(t, err)

newSignRequestDisabledTxReceipt, err := bind.WaitMined(ctx, evmClient, newSignRequestDisabledTx)
require.NoError(t, err)

newSignRequestDisabledEvents, err := checks.GetParsedEventsOnly(newSignRequestDisabledTxReceipt, iWardenClient.ParseNewSignRequest)
require.NoError(t, err)

require.Len(t, newSignRequestDisabledEvents, 1)
require.Equal(t, newSignRequestDisabledEvents[0].Id, uint64(2))
require.Equal(t, newSignRequestDisabledEvents[0].KeyId, uint64(1))
require.Equal(t, newSignRequestDisabledEvents[0].Creator, alice.EthAddress(t))
require.Equal(t, newSignRequestDisabledEvents[0].BroadcastType, uint8(types.BroadcastType_BROADCAST_TYPE_DISABLED))

actions5, err := iActClient.Actions(alice.CallOps(t), act.TypesPageRequest{})

require.NoError(t, err)
require.Len(t, actions5.Actions, 5)

signRequestsDisabled, err = iWardenClient.SignRequests(alice.CallOps(t), warden.TypesPageRequest{}, 1, uint8(types.SignRequestStatus_SIGN_REQUEST_STATUS_PENDING), uint8(types.BroadcastType_BROADCAST_TYPE_DISABLED)+1)

require.NoError(t, err)
require.Len(t, signRequestsDisabled.SignRequests, 1)
require.Equal(t, warden.SignRequest{
Id: 2,
Creator: alice.EthAddress(t),
KeyId: 1,
Status: uint8(types.SignRequestStatus_SIGN_REQUEST_STATUS_PENDING),
EncryptionKey: []byte{},
DataForSigning: []byte{30, 134, 120, 103, 230, 84, 237, 151, 116, 242, 69, 17, 228, 215, 27, 180, 86, 107, 152, 98, 133, 215, 201, 146, 4, 157, 189, 118, 13, 42, 35, 142},
DeductedKeychainFees: []warden.TypesCoin{},
Result: []byte{},
BroadcastType: uint8(types.BroadcastType_BROADCAST_TYPE_DISABLED),
}, signRequests.SignRequests[0])
}, signRequestsDisabled.SignRequests[0])

signRequestsAutomatic, err := iWardenClient.SignRequests(
signRequestsAutomatic, err = iWardenClient.SignRequests(
alice.CallOps(t),
warden.TypesPageRequest{},
1,
uint8(types.SignRequestStatus_SIGN_REQUEST_STATUS_PENDING),
uint8(types.BroadcastType_BROADCAST_TYPE_AUTOMATIC))
require.Len(t, signRequestsAutomatic.SignRequests, 0)
uint8(types.BroadcastType_BROADCAST_TYPE_AUTOMATIC+1))
require.NoError(t, err)
require.Len(t, signRequestsAutomatic.SignRequests, 1)

signRequests, err := iWardenClient.SignRequests(
alice.CallOps(t),
warden.TypesPageRequest{},
1,
uint8(types.SignRequestStatus_SIGN_REQUEST_STATUS_PENDING),
0)
require.NoError(t, err)
require.Len(t, signRequests.SignRequests, 2)

// updateKey
newTemplateTx, err := iActClient.NewTemplate(
Expand Down Expand Up @@ -302,10 +369,10 @@ func (c *Test_WardenPrecompileAction) Run(t *testing.T, ctx context.Context, bui
require.Equal(t, updateKeyEvents[0].ApproveTemplateId, uint64(1))
require.Equal(t, updateKeyEvents[0].RejectTemplateId, uint64(1))

actions5, err := iActClient.Actions(alice.CallOps(t), act.TypesPageRequest{})
actions6, err := iActClient.Actions(alice.CallOps(t), act.TypesPageRequest{})

require.NoError(t, err)
require.Len(t, actions5.Actions, 5)
require.Len(t, actions6.Actions, 6)

key, err := iWardenClient.KeyById(
alice.CallOps(t),
Expand Down Expand Up @@ -351,10 +418,10 @@ func (c *Test_WardenPrecompileAction) Run(t *testing.T, ctx context.Context, bui
require.Equal(t, updateSpaceEvents[0].ApproveSignTemplateId, uint64(1))
require.Equal(t, updateSpaceEvents[0].RejectSignTemplateId, uint64(1))

actions6, err := iActClient.Actions(alice.CallOps(t), act.TypesPageRequest{})
actions7, err := iActClient.Actions(alice.CallOps(t), act.TypesPageRequest{})

require.NoError(t, err)
require.Len(t, actions6.Actions, 6)
require.Len(t, actions7.Actions, 7)

space, err := iWardenClient.SpaceById(alice.CallOps(t), 1)

Expand Down
2 changes: 1 addition & 1 deletion warden/x/warden/keeper/query_sign_requests.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func (k Keeper) SignRequests(goCtx context.Context, req *types.QuerySignRequests
return false, nil
}

if value.BroadcastType != req.BroadcastType {
if req.OptionalBroadcastType != nil && value.BroadcastType != req.GetBroadcastType() {
return false, nil
}

Expand Down
Loading

0 comments on commit d7f79f9

Please sign in to comment.