fix: collect all responses from authz/MsgExec #9538
Conversation
Codecov Report
@@ Coverage Diff @@
## master #9538 +/- ##
=======================================
Coverage 60.58% 60.59%
=======================================
Files 589 589
Lines 37228 37222 -6
=======================================
- Hits 22556 22554 -2
+ Misses 12728 12724 -4
Partials 1944 1944
|
|
does this req a changelog entry? also a test to check the amt of results returned would be cool |
proto/cosmos/authz/v1beta1/tx.proto
Outdated
| @@ -40,7 +40,7 @@ message MsgGrant { | |||
|
|
|||
| // MsgExecResponse defines the Msg/MsgExecResponse response type. | |||
| message MsgExecResponse { | |||
| cosmos.base.abci.v1beta1.Result result = 1; | |||
| repeated cosmos.base.abci.v1beta1.Result results = 1; | |||
There was a problem hiding this comment.
Let's get rid of Result and just use Any. Result could be non deterministic because it contains log
There was a problem hiding this comment.
Or if we don't have the Any then just repeated bytes with the data field
There was a problem hiding this comment.
Would that affect #9522? I was planning on using the result field to emit the executed events
There was a problem hiding this comment.
We just need to emit them with the event emitter not in the result
There was a problem hiding this comment.
Result could be non deterministic because it contains log
I thought the return value is not included in a block.
Or if we don't have the Any
IMHO, Any should be used only if we have a well defined interface.
There was a problem hiding this comment.
I wonder if it makes sens to include events as well - this way, other modules calling this module will be able to inspect events. WDYT?
There was a problem hiding this comment.
Data is included in the block but events and log are non deterministic. At least that's my understanding. So we should include just data and not events or log. Events will need another method for introspection I believe.
There was a problem hiding this comment.
I will change to repeated bytes responses (where each response is the data attribute).
NOTE: I don't see where we add return values in a block. The Data in a Block is not response - it's a set of serialized transactions: https://github.com/tendermint/tendermint/blob/a6b30faf35fe28b30401e902bcf2498f092d2cf6/types/block.go#L1001
Header also doesn't have any commitment to responses nor events.
[digression]
So, since event commitment is not included in a block, then a state machines must not make any decision based on events fired by other service / module. We can run a modified node with all events removed (all code triggering events) and will be still compatible with the rest of the network.
There was a problem hiding this comment.
Well events fired in the state machine are effectively deterministic, but from tendermint's perspective they aren't. Look at abci.ResponseDeliverTx
@technicallyty - we don't have changelog for 0.43-beta, so we don't make a changelog for diff between 0.43-beta and 0.43 |
robert-zaremba
added
the
A:automerge
Description
Closes: #9536
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!to the type prefix if API or client breaking changeCHANGELOG.mdReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
!in the type prefix if API or client breaking change