Standardize emission events naming#900
Conversation
|
The latest Buf updates on your PR. Results from workflow Buf Linter / buf (pull_request).
|
xmariachi
left a comment
There was a problem hiding this comment.
- Please use "nonce" instead of "nonce_block_height".
- Needs a rebase
3c3b61b to
7508b33
Compare
|
Also - this should come with changes in the consumers, especially producer, right? |
xmariachi
left a comment
There was a problem hiding this comment.
Left a comment about deprecating field / pushing version
| ActorType actor_type = 1; | ||
| uint64 topic_id = 2; | ||
| int64 block_height = 3; | ||
| int64 nonce = 3; |
There was a problem hiding this comment.
The new field on an existing needs to have a new unused number at the end (6 in this case), adding the number 3 (and the name) as reserved. However if we keep this v9 version, this block_height field and its number 3 should be kept, but as "deprecated" to keep compat with previous v9 events. If we maintain v9 version, they should be compatible.
... But maybe we want to generate a new version in general for all of these changes? This change in particular does not require it, but we are changing event names, which makes the old events not being emitted anymore, making it a breaking change to consumers.
So I think we should just add a v10?
Purpose of Changes and their Description
This PR standardizes emission event naming conventions to make nonce vs tx block height explicit and align "Set" suffix usage.
Changes
1. Nonce field standardization
nonce(fromblock_heightornonce_block_height) in events where the field represents the work-cycle nonce.block_height_txfor transaction block height where applicable (for example,EventRewardsSettled).block_heightfield from last-commit and previous-percentage events (reserved field numbers and names).2. "Set" suffix alignment
Affected Events
Nonce field rename to
nonceEventScoresSetblock_heightnonceEventRewardsSettledblock_heightnonceEventForecastTaskScorenonce_block_heightnonceEventListeningCoefficientsSetblock_heightnonceEventInfererNetworkRegretSetblock_heightnonceEventForecasterNetworkRegretSetblock_heightnonceEventNaiveInfererNetworkRegretSetblock_heightnonceEventTopicInitialRegretblock_heightnonceEventTopicInitialEmaScoreblock_heightnonceEventRegretStdNormblock_heightnonceEventInfererWeightsSetblock_heightnonceEventForecasterWeightsSetblock_heightnonceEventActiveReputersSetnonce_block_heightnonceEventActiveInferersSetnonce_block_heightnonceEventActiveForecastersSetnonce_block_heightnonceEventNetworkInferenceInfererWeightsSetnonce_block_heightnonceEventNetworkInferenceForecasterWeightsSetnonce_block_heightnonceEventNetworkInferenceInfererRegretsUsedSetnonce_block_heightnonceEventNetworkInferenceForecasterRegretsUsedSetnonce_block_heightnonceEventWorkerSubmissionWindowOpenednonce_block_heightnonceEventWorkerSubmissionWindowClosednonce_block_heightnonceEventReputerSubmissionWindowOpenednonce_block_heightnonceEventReputerSubmissionWindowClosednonce_block_heightnonceblock_heightfield removed (reserved)EventWorkerLastCommitblock_height(reserved)EventReputerLastCommitblock_height(reserved)EventPreviousPercentageRewardToStakedReputersblock_height(reserved)"Set" suffix removal (single value events)
EventParamsSetEventParamsEventForecastTaskScoreSetEventForecastTaskScoreEventWorkerLastCommitSetEventWorkerLastCommitEventReputerLastCommitSetEventReputerLastCommitEventTopicInitialRegretSetEventTopicInitialRegretEventTopicInitialEmaScoreSetEventTopicInitialEmaScoreEventRegretStdNormSetEventRegretStdNormEventPreviousPercentageRewardToStakedReputersSetEventPreviousPercentageRewardToStakedReputersAre these changes tested and documented?
Unreleasedsection ofCHANGELOG.md?