cosmos-sdk
cosmos-sdk copied to clipboard
cosmos
[Bug]: Authz MsgExecResponse.Results encoding
Is there an existing issue for this?
- [x] I have searched the existing issues
What happened?
Background
While working on https://github.com/pokt-network/poktroll/pull/826, I experienced issues when adding test assertions around the MsgExecResponse.Results field.
I've outlined the codepaths that I followed to bulid context and formulate my expectation in a comment in the test in question:
// TODO_INVESTIGATE: It seems like the response objects are encoded in
// an unexpected way. It's unclear whether this is the result of being
// executed via authz. Looking at the code, it seems like authz utilizes the
// sdk.Result#Data field of the result which is returned from the message handler.
// These result byte slices are accumulated for each message in the MsgExec and
// set on the MsgExecResponse#Results field.
//
// - https://github.com/cosmos/cosmos-sdk/blob/v0.50.9/x/authz/keeper/msg_server.go#L120
// - https://github.com/cosmos/cosmos-sdk/blob/v0.50.9/x/authz/keeper/keeper.go#L166
// - https://github.com/cosmos/cosmos-sdk/blob/v0.50.9/baseapp/msg_service_router.go#L55
// - https://github.com/cosmos/cosmos-sdk/blob/v0.50.9/baseapp/msg_service_router.go#L198
// - https://github.com/cosmos/cosmos-sdk/blob/v0.50.9/types/result.go#L213
//
// I (@bryanchriswhite) would've expected the following to work, but it does not:
updateResValue := reflect.New(reflect.TypeOf(moduleCfg.ParamsMsgs.MsgUpdateParamResponse))
// NB: using proto.Unmarshal here because authz seems to use
// proto.Marshal to serialize each message response.
err = proto.Unmarshal(updateResBz, updateResValue.Interface().(cosmostypes.Msg))
require.NoError(t, err)
updateResParamValue := updateResValue.Elem().FieldByName("Params").Elem().FieldByName(fieldName)
require.Equal(t, fieldExpectedValue.Interface(), updateResParamValue.Interface())
This snippet above is an excerpt from a test which can be summarized like so:
Given some cosmos-sdk appchain modules, "Modules"
And each module is configured with some authority, "Authority"
And each module supports a distinct `MsgUpdateParam` message
And an authz grant exists which authorizes "Authorized" to execute any module's `MsgUpdateParam` message on behalf of "Authority"
When an authz `MsgExec` which includes a `MsgUpdateParam` message is sent from "Authorized"
Then the `MsgExecResponse.Results` should be a slice of serialized message response objects corresponding to the messages which were executed in the `MsgExec` (e.g. `MsgUpdateParamResponse`)
When the params are queried next
Then the params should have been updated
Relevant modules
Currently, only some modules have any params. Support for MsgUpdateParam is limted to modules which do:
Due diligence
Each module does indeed include a Params field in its respective MsgUpdateParamResponse type, assign a non-zero-value value to it in their respective message handlers, and register the correct type as the return for the UpdateParam rpc definition:
-
sharedtypes.MsgUpdateParamResponse -
prooftypes.MsgUpdateParamResponse -
servicetypes.MsgUpdateParamResponse
Failure modes
The final assertions from the test summary hold:
...
When the params are queried next
Then the params should have been updated
It is only the assertion(s) around the MsgExecResponse.Results which are problematic:
...
Then the `MsgExecResponse.Results` should be a slice of serialized message response objects corresponding to the messages which were executed in the `MsgExec` (e.g. `MsgUpdateParamResponse`)
...
wrong wireType
In the case of the shared module, an error is returned when attempting to decode the first (and only) serialized message response (i.e. MsgExec.Results[0]):
=== RUN TestUpdateParamSuite/TestAuthorizedMsgUpdateParamSucceeds/shared_NumBlocksPerSession
update_param_test.go:128:
Error Trace: /home/bwhite/Projects/pokt/poktroll/tests/integration/params/update_param_test.go:128
Error: Received unexpected error:
proto: wrong wireType = 2 for field NumBlocksPerSession
Test: TestUpdateParamSuite/TestAuthorizedMsgUpdateParamSucceeds/shared_NumBlocksPerSession
--- FAIL: TestUpdateParamSuite/TestAuthorizedMsgUpdateParamSucceeds/shared_NumBlocksPerSession (0.02s)
Unexpected decoded values
In the case of the proof and service modules, decoding as described doesn't cause an error. Instead, the first service and proof module params values seem to hold values that contain the entire params struct data (i.e. a serialize Param object, rather than a particular field value thereof):
=== RUN TestUpdateParamSuite/TestAuthorizedMsgUpdateParamSucceeds/proof_RelayDifficultyTargetHash
update_param_test.go:130:
Error Trace: /home/bwhite/Projects/pokt/poktroll/tests/integration/params/update_param_test.go:130
Error: Not equal:
expected: []byte{0x0, 0x0, 0x0, 0x0, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff}
actual : []byte{0xa, 0x20, 0x0, 0x0, 0x0, 0x0, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x15, 0x0, 0x0, 0x80, 0x3e, 0x1a, 0x11, 0xa, 0x5, 0x75, 0x70, 0x6f, 0x6b, 0x74, 0x12, 0x8, 0x32, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x22, 0x12, 0xa, 0x5, 0x75, 0x70, 0x6f, 0x6b, 0x74, 0x12, 0x9, 0x33, 0x32, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x2a, 0x10, 0xa, 0x5, 0x75, 0x70, 0x6f, 0x6b, 0x74, 0x12, 0x7, 0x31, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30}
Diff:
--- Expected
+++ Actual
@@ -1,4 +1,8 @@
-([]uint8) (len=32) {
- 00000000 00 00 00 00 ff ff ff ff ff ff ff ff ff ff ff ff |................|
+([]uint8) (len=96) {
+ 00000000 0a 20 00 00 00 00 ff ff ff ff ff ff ff ff ff ff |. ..............|
00000010 ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff |................|
+ 00000020 ff ff 15 00 00 80 3e 1a 11 0a 05 75 70 6f 6b 74 |......>....upokt|
+ 00000030 12 08 32 30 30 30 30 30 30 30 22 12 0a 05 75 70 |..20000000"...up|
+ 00000040 6f 6b 74 12 09 33 32 30 30 30 30 30 30 30 2a 10 |okt..320000000*.|
+ 00000050 0a 05 75 70 6f 6b 74 12 07 31 30 30 30 30 30 30 |..upokt..1000000|
}
Test: TestUpdateParamSuite/TestAuthorizedMsgUpdateParamSucceeds/proof_RelayDifficultyTargetHash
--- FAIL: TestUpdateParamSuite/TestAuthorizedMsgUpdateParamSucceeds/proof_RelayDifficultyTargetHash (0.03s)
Expected :[]byte{0x0, 0x0, 0x0, 0x0, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff}
Actual :[]byte{0xa, 0x20, 0x0, 0x0, 0x0, 0x0, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x15, 0x0, 0x0, 0x80, 0x3e, 0x1a, 0x11, 0xa, ...
E.g.,
proof_RelayDifficultyTargetHash
=== RUN TestUpdateParamSuite/TestAuthorizedMsgUpdateParamSucceeds/service_AddServiceFee
update_param_test.go:130:
Error Trace: /home/bwhite/Projects/pokt/poktroll/tests/integration/params/update_param_test.go:130
Error: Not equal:
expected: &types.Coin{Denom:"upokt", Amount:math.Int{i:(*big.Int)(0xc00123a120)}}
actual : &types.Coin{Denom:"\n\x05upokt\x12\n1000000001", Amount:math.Int{i:(*big.Int)(nil)}}
Diff:
--- Expected
+++ Actual
@@ -1,10 +1,5 @@
(*types.Coin)({
- Denom: (string) (len=5) "upokt",
+ Denom: (string) (len=19) "\n\x05upokt\x12\n1000000001",
Amount: (math.Int) {
- i: (*big.Int)({
- neg: (bool) false,
- abs: (big.nat) (len=1) {
- (big.Word) 1000000001
- }
- })
+ i: (*big.Int)(<nil>)
}
Test: TestUpdateParamSuite/TestAuthorizedMsgUpdateParamSucceeds/service_AddServiceFee
--- FAIL: TestUpdateParamSuite/TestAuthorizedMsgUpdateParamSucceeds/service_AddServiceFee (0.04s)
Expected :&types.Coin{Denom:"upokt", Amount:math.Int{i:(*big.Int)(0xc00123a120)}}
Actual :&types.Coin{Denom:"\n\x05upokt\x12\n1000000001", Amount:math.Int{i:(*big.Int)(nil)}}
E.g.,
service_AddServiceFee
Nil or zero-value field values
In the case of the proof module, it seems that when asserting on any field other than the first, the resulting concrete object is its type's zero-value or a nil pointer. This can be observed by adding the following line above the failing assertion (in the excerpt above), which causes 4 of the proof module's tests cases to fail for this reason:
switch updateResParamValue.Kind() {
case reflect.Ptr:
require.False(t, updateResParamValue.IsNil())
default:
require.False(t, updateResParamValue.IsZero())
}
This observation might apply generally as the service module's response params only include one value which results in the second failure mode. It would stand to reason that if additional fields were present, they would result in this failure mode.
Cosmos SDK Version
0.50.9
How to reproduce?
git clone https://github.com/pokt-network/poktroll
# See: https://dev.poktroll.com/develop/developer_guide/quickstart#0-install-dependencies
make install_ci_deps
make go_develop
# Uncomment the assertions from the excerpt above; i.e., `tests/integration/params/update_param_test.go:124-130.
go test -v ./tests/integration/params/update_param_test.go
