Skip to content

[Retroactive event changes] Clarify bond events #check-lint-warning

Multipartite requested to merge Multi/retroactively-clarify-bond-events into develop

[Retroactive event changes]

2024-07-08 update: After rebasing, this now includes block export changes,
for example for deposit/deposit.json and mimir/node_mimir.json .

bond_cost and any bond_reward events have empty bond_address fields as they are between the network and the node itself;
in regression tests, it's relevant to keep in mind that the nodes are their own operator addresses,
thus why for bond-address-specific bond_paid and any bond_returned events the bond_address field is the same as the node_address field.

1

2

This is intended to close #1366
'Bond event readability'.

If merged, Midgard code modification to interpret a string bond_type will be needed.
(As well as to interpret added node_address and bond_address fields.)
|
For the string bond_type specifically, here
keeping the one-byte logic to work for both THORNode versions during the switchover,
but replacing
return fmt.Errorf("malformed bond_type: should be a single byte but value is %q", attr.Value)
with
e.BondType = attr.Value
like many of the other fields in that switch.

THORNode changes:

1: Never .String() the entire NodeAccount for a bond event, only its NodeAddress.

2: Use .String() rather than string() for the EventBond Events, so that the bond_type displays for instance as bond_cost rather than \u0003.

3: Add node_address and bond_address fields,
so that each bond event unambiguously communicates which node address it is for
(as multiple nodes can be bonded by the same bond provider)
and (when for a single bond like a BondPaid or BondReturned) which bond address it is for
(as a single node can be bonded by multiple bond providers).
|
|
Approach:
Have NewEventBond take a NodeAccount (or pointer to NodeAccount to reduce copying) and AccAddress argument (two different types to be unambiguous).
For when the node_operator_address (nodeAccount.BondAddress) is to be used, use nodeAccount.BondAddress.AccAddress().
|
For when a BondReward or BondCost is shared between all bonds,
use nil as the bondAddress argument for NewEventBond.

For proof-of-concept sync testing before a full NewEventBond function change
(keeping the NewEventBond arguments, but for a historical block trying the proposed new NewEventBond code
instead of calling NewEventBond)
I have used the Nine Realms pruned-snapshot of height 12465194 for testing the effect on the bond event in block 12466865, 1,671 blocks later.
|
Specifically:
Sync with release code and view with release querier.
View with proof-of-concept commit's querier (which has the changes to the protobuf and Events).
Sync with proof-of-concept commit (confirming that there's no sync failure) and view with proof-of-concept querier.
|
As block 12466865's bond event is from a MsgMimir and block 12466865 used network version 1.120.0, the proof-of-concept function change would be in handler_mimir.go's handleV112.
For easy effect interpretation, the base commit used for comparison would be commit a10e104b from the currently-unmerged !3121 (merged)
'[api] Add /thorchain/block API'.

Proof of concept commit for block 12466865 from pruned snapshot height 12465194 (compared to commit a10e104b):
https://gitlab.com/thorchain/thornode/-/commits/dd4ac6bd

There was no sync failure.

Endpoint URL used for the fullnode: http://localhost:1317/thorchain/block?height=12466865

2023-09-17_a10e104b-code_a10e104b-querier_block_12466865.json

2023-09-17_a10e104b-code_dd4ac6bd-querier_block_12466865.json

2023-09-17_dd4ac6bd-code_dd4ac6bd-querier_block_12466865.json

Bond event for both {a10e104b-code a10e104b-querier} and {a10e104b-code dd4ac6bd-querier}:

          {
            "amount": "2000000",
            "bond_type": "\u0003",
            "chain": "",
            "coin": "",
            "from": "",
            "id": "0000000000000000000000000000000000000000000000000000000000000000",
            "memo": "",
            "to": "node:thor1yedw5cuz4l93wyj58wdn7s7z8qppfvusl3c66p\nstatus:Active\nnode pubkeys:\n\tsecp256k1: thorpub1addwnpepqdyxq6wpxlejdkdkf83emm3vndnqsa0hmth09f037zp3pdl8tefag09f9ac\n\ted25519: thorpub1zcjduepqva9v9gc7ewwh2jc7av6xkwfjlwqsa7qlntufdhtjvwwf7esnu7ssr3hwl2\n\nvalidator consensus pub key:thorcpub1zcjduepqppewk76dqa4v4chep4wfnhyhx05jhr5evfakagcx6azp98dgz82sp7zm32\nbond:102499118625318\nversion:1.120.0\nbond address:thor1j82t8zu0ht8ha8u53f5h4yhvz326f2yj8h073r\nrequested to leave:false\n",
            "type": "bond"
          }

Bond event for {dd4ac6bd-code dd4ac6bd-querier}:

          {
            "amount": "2000000",
            "bond_address": "",
            "bond_type": "bond_cost",
            "chain": "",
            "coin": "",
            "from": "",
            "id": "0000000000000000000000000000000000000000000000000000000000000000",
            "memo": "",
            "node_address": "thor1yedw5cuz4l93wyj58wdn7s7z8qppfvusl3c66p",
            "to": "thor1yedw5cuz4l93wyj58wdn7s7z8qppfvusl3c66p",
            "type": "bond"
          }

Feedback is welcome.

The above proof of concept has now been extended in this RM to all NewEventBond calls in the THORNode code,
by altering the NewEventBond function and adding *NodeAccount and cosmos.AccAddress arguments to every instance of NewEventBond being called.

(2024-01-17 update: Some time has passed, so rebasing to apply to some functions newly added to old-version files.)

Edited by Multipartite

Merge request reports