THORChain doesn't emit a "version" event when the network version changes
Individual nodes emit "set_version" events when they set their versions. When all Active nodes have switched to a new version, the THORChain network version (and thus code behaviour) changes, as reflected in the /version endpoint.
New Standby nodes can continue to emit "set_version" events after the network version has changed, or the version can change when older-version nodes are churned out and leave only newer-version nodes Active.
Example:
Block 9368120 (note the "set_version" event): version 1.104.0 .
Block 9368121, first block of the v1.105.0 code: version 1.104.0 .
Block 9496773 has a later "set_version" event for version 1.105.0 .
Finding when a version has changed can thus be a matter of set_version event binary search with a Flipside query,
paying attention to churns in which which nodes are marked to churn out from low version,
or as a fallback plan doing a binary search with the /version endpoint.
As the network version changes infrequently (about once per month or week), emitting one "version" event every time the network version changed would not represent meaningful event bloat.
I would welcome being able to get the new-version block heights directly from emitted events,
and/but I am prompted to make this Issue by the prospect of Midgard being able to keep track of THORNode version changes
and by doing so maintain synchrony with THORNode behaviour changes.
As a specific example:
Block 1752717: version 0.64.0 .
Block 2104916: still version 0.64.0 .
Block 2104917: version 0.67.0 .
As seen in v0.67.0's handler_withdraw.go,
if !inboundAsset.IsEmpty() {
assetAmount = assetAmount.Add(inboundAsset.Amount)
}
is present in handleV55
and not in handleCurrent
(pointed to by handleV65
). In other words, that behaviour wasn't present in version 0.64.0 and was implemented from block 2104917 onwards.
(The MR was !1834 (merged), specifically for Issue #1052 (closed) '[ADD] Stop withdraw send back the inbound asset'.)
In Midgard's record.go:
// Logic for withdraw changed since start of chaosnet 2021-04.
//
// Background: In order to initiate a withdraw the user needs to send a transaction with a memo.
// On many asset chains one can't send 0 value, therefore they often send a small amount
// of asset (e.g. 1e-8). The value sent in by the user is shown in withdraw.coin
// (unstake.asset_e8)
//
// New logic: keeps withdraw.coin in the wallets but it doesn't increment depths with it.
//
// Old logic: Pools don't keep this amount, it's forwarded back to the user and included in
// the EmitAssetE8. Therefore pool depth decreases with less then the EmitAssetE8
// This was not applied for rune or assets different then the native asset of the chain:
// - for Rune there is no minimum amount, if some rune is sent it's kept as donation.
// - when for non chain native assets (e.g. ETH.USDT) the EmitAssetE8 could not have contained
// the coin sent in.
if meta.BlockHeight < withdrawCoinKeptHeight {
if e.AssetE8 != 0 && string(e.Pool) == string(e.Asset) {
r.AddPoolAssetE8Depth(e.Pool, e.AssetE8)
}
}
Importantly, that withdrawCoinKeptHeight
is defined (for Mainnet) here in correctmainnet.go:
withdrawCoinKeptHeight = 1970000
Midgard to my knowledge currently cannot be told beforehand
"THORNode behaviour will change in version X, so respond to events in way A for THORNode version lower than X and in way B for THORNode version equal or higher than X."
Since the exact block height of a new version's implementation typically cannot be known beforehand,
Midgard can only be told the exact block height of a THORNode version in a later Midgard release (until which it will have discrepancies)
or be given an approximate block height beforehand (in which case it will also have discrepancies).
In either case discrepancies may need to be manually cancelled out.
I currently imagine ideal behaviour being for Flipside, front-ends and otherwise using Midgard to update their Midgard release before a THORNode release, then upon the THORNode release the Midgard instances continuing to give data that matched THORNode's without a period of giving inaccurate data.
As managers.go's BeginBlock
already checks for a new network version in deciding whether to recreate managers, I currently propose that this function also include a "version" event emission, perhaps immediately after the EventManager has been created.
I initially thought to only use mgr's version check, perhaps not emitting events when mgr had no stored version (.Equals(semver.Version{})
),
though/but I considered this would have an edge case if a node's first block was the first block of a ne version, thus failing to emit.
|
My current preference is to use the keeper's stored version used in the same function,
there for the sake of version querying without rederiving,
which I hope should be a reliably consistent indication of version increase.
(Particularly in regards to intuitively keeping version emission consistent with the /version endpoint.)