feat(cosmic-swingset): Leave inbound actions in vstorage #7115
Conversation
Datadog ReportBranch report: ✅ |
|
PTAL @michaelfig |
Datadog ReportBranch report: ✅ |
| type ActionContext struct { | ||
| BlockHeight int64 `json:"blockHeight"` | ||
| TxHash string `json:"txHash"` | ||
| MsgIdx int `json:"msgIdx"` |
There was a problem hiding this comment.
Comment on where this index is meaningful. Index of the cosmos message in its transaction? In the block? Or is this the index of a swingset message?
| if !found { | ||
| return errors.New("could not find max inboundQueue size in params") | ||
| } | ||
| inboundMempoolQueueMax := inboundQueueMax / 2 |
There was a problem hiding this comment.
This is a separate entry: types.QueueInboundMempool. Please correct any comment or doc which says it's always half of QueueInbound.
There was a problem hiding this comment.
I don't understand, which docs or comment says that?
There was a problem hiding this comment.
Let me turn my request in to a question instead - what made you think that the inbound mempool queue max should be half of the inbound queue max?
There was a problem hiding this comment.
That's what the existing behavior that I'm porting from JS is.
While we have a params.QueueMax value for QueueInbound, we don't have an explicit param value for QueueInboundMempool, which is hardcoded and documented as 50% of the QueueInbound value.
However we do need to compute a a value for the QueueInboundMempool of the state.QueueAllowed. That value is indeed only going to be 50% of the allowed QueueInbound if the actionQueue is empty. However I don't believe there is any claim anywhere that the allowed QueueInboundMempool is always 50% of the allowed QueueInbound.
| if !found { | ||
| return errors.New("could not find max inboundQueue size in params") | ||
| } | ||
| inboundMempoolQueueMax := inboundQueueMax / 2 |
There was a problem hiding this comment.
Let me turn my request in to a question instead - what made you think that the inbound mempool queue max should be half of the inbound queue max?
| if !found { | ||
| return errors.New("could not find max inboundQueue size in params") | ||
| } | ||
| inboundMempoolQueueMax := inboundQueueMax / 2 |
mhofman
force-pushed
the
mhofman/7102-keep-action-queue
branch
from
5115f02 to
c14f611
Compare
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #7115 +/- ##
==========================================
- Coverage 70.64% 70.58% -0.06%
==========================================
Files 447 442 -5
Lines 85082 84613 -469
Branches 3 3
==========================================
- Hits 60107 59725 -382
+ Misses 24909 24820 -89
- Partials 66 68 +2
... and 13 files with indirect coverage changes |
Use context to generate inboundNum
getKeys -> getNextKey throwing stub
Actual has & delete
Prevents updating the queue head if queue started empty
mhofman
force-pushed
the
mhofman/7102-keep-action-queue
branch
from
c14f611 to
a32299d
Compare
closes: #7102
refs: agoric-labs/cosmos-sdk#300 (pre-requisite)
Best reviewed commit-by-commit
Description
Remove the inboundQueue from swingstore, keep pending inbound actions in the vstorage backed
actionQueue.To keep functionality intact, create a new context for actions at the cosmos level, which includes
blockHeight,txHashandmsgIdx, which in the future will allow better integrated telemetry. Use that context to generate theinboundNumidentifier.InboundAllowed is now computed in the golang side, meaning cosmic-swingset no longer has a say in how many actions should be pending. If we need to in the future, we can re-introduce data that cosmic-swingset can return to cosmos to include in the decision.
Security Considerations
None
Scaling Considerations
This will potentially make 2 performance related upcoming features slightly more complicated:
PushActioncan pick the right queue instead of leaving that to cosmic-swingsetDocumentation Considerations
None
Testing Considerations
Manually tested by dropping the computron limit and maxAllowedQueue size, and verified with the loadgen that inboundQueue limits still function correctly.