Chain Reader config improvements

[jmank88]

• core/services/job: remove JSONConfig.Bytes hack
• Shorten median chain reader smoke test abi
• Improve ReadType UX by changing the enum to string

[github-actions]

I see that you haven't updated any README files. Would it make sense to do so?

[github-actions]

I see that you haven't updated any CHANGELOG files. Would it make sense to do so?

[ilija42]

This doesn't work, it just ends up sending bytes representation of json in a string to get unmarshalled into RelayConfig.
for eg. codec looks like this
"codec\":\"[123 34 99 104 97 105 110 67 111 100 101 99 67 111 110 102 105 103 34 32 58 123 34 77 101 100 105 97 110 82 101 112 111 114 116 34 58 123 34 84 121 112 101 65 98 105 34 58 32 34 91 123 92 34 78 97 109 101 92 34 58 32 92 34 84 105 109 101 115 116 97 109 112 92 34 44 92 34 84 121 112 101 92 34 58 32 92 34 117 105 110 116 51 50 92 34 125 44 123 92 34 78 97 109 101 92 34 58 32 92 34 79 98 115 101 114 118 101 114 115 92 34 44 92 34 84 121 112 101 92 34 58 32 92 34 98 121 116 101 115 51 50 92 34 125 44 123 92 34 78 97 109 101 92 34 58 32 92 34 79 98 115 101 114 118 97 116 105 111 110 115 92 34 44 92 34 84 121 112 101 92 34 58 32 92 34 105 110 116 49 57 50 91 93 92 34 125 44 123 92 34 78 97 109 101 92 34 58 32 92 34 74 117 101 108 115 80 101 114 70 101 101 67 111 105 110 92 34 44 92 34 84 121 112 101 92 34 58 32 92 34 105 110 116 49 57 50 92 34 125 93 34 125 125 125]\"

fails here to be exact

[jmank88]

Debugging TestIntegration_OCR2/chain-reader reveals the real problem: we use type RelayConfig struct inside of (o *RelayOpts) RelayConfig(), which expects chainReader and codec to be JSON objects, but we are setting them as encoded JSON string in the TOML. The other fields don't have this problem because they are used as-is for both TOML and JSON. Why are we doing this usual approach? We already bridge TOML to JSON - why don't we continue to use that same pattern?

IMHO following the existing style leads to a much better user experience by producing more readable code and we can completely avoid escpaing JSON:

[relayConfig]
chainID = 1337
fromBlock = 42
[relayConfig.chainReader.chainContractReaders.median]
contractABI = '''
[
  {
    "inputs": [
      {
        "internalType": "contractLinkTokenInterface",
        "name": "link",
        "type": "address"
      }
    ]
  }
]
'''
[relayConfig.chainReader.chainContractReaders.median.chainReaderDefinitions.LatestTransmissionDetails]
chainSpecificName = "latestTransmissionDetails"
readType = 0
[[relayConfig.chainReader.chainContractReaders.median.chainReaderDefinitions.LatestTransmissionDetails.output_modifications]]
type = "rename"
fields = {
  LatestAnswer_ = "LatestAnswer",
  LatestTimestamp_ = "LatestTimestamp"
}
[relayConfig.chainReader.chainContractReaders.median.chainReaderDefinitions.LatestRoundRequested]
chainSpecificName = "RoundRequested"
"params": {
"requester": ""
},
"readType": 1
[relayConfig.codec.chainCodecConfig.MedianReport]
TypeAbi = '''
[
  {
    "Name": "Timestamp",
    "Type": "uint32"
  },
  {
    "Name": "Observers",
    "Type": "bytes32"
  },
  {
    "Name": "Observations",
    "Type": "int192[]"
  },
  {
    "Name": "JuelsPerFeeCoin",
    "Type": "int192"
  }
]
'''

[ilija42]

Debugging TestIntegration_OCR2/chain-reader reveals the real problem: we use type RelayConfig struct inside of (o *RelayOpts) RelayConfig(), which expects chainReader and codec to be JSON objects, but we are setting them as encoded JSON string in the TOML. The other fields don't have this problem because they are used as-is for both TOML and JSON. Why are we doing this usual approach? We already bridge TOML to JSON - why don't we continue to use that same pattern?

IMHO following the existing style leads to a much better user experience by producing more readable code and we can completely avoid escpaing JSON:

[relayConfig]
chainID = 1337
fromBlock = 42
[relayConfig.chainReader.chainContractReaders.median]
contractABI = '''
[
  {
    "inputs": [
      {
        "internalType": "contractLinkTokenInterface",
        "name": "link",
        "type": "address"
      }
    ]
  }
]
'''
[relayConfig.chainReader.chainContractReaders.median.chainReaderDefinitions.LatestTransmissionDetails]
chainSpecificName = "latestTransmissionDetails"
readType = 0
[[relayConfig.chainReader.chainContractReaders.median.chainReaderDefinitions.LatestTransmissionDetails.output_modifications]]
type = "rename"
fields = {
  LatestAnswer_ = "LatestAnswer",
  LatestTimestamp_ = "LatestTimestamp"
}
[relayConfig.chainReader.chainContractReaders.median.chainReaderDefinitions.LatestRoundRequested]
chainSpecificName = "RoundRequested"
"params": {
"requester": ""
},
"readType": 1
[relayConfig.codec.chainCodecConfig.MedianReport]
TypeAbi = '''
[
  {
    "Name": "Timestamp",
    "Type": "uint32"
  },
  {
    "Name": "Observers",
    "Type": "bytes32"
  },
  {
    "Name": "Observations",
    "Type": "int192[]"
  },
  {
    "Name": "JuelsPerFeeCoin",
    "Type": "int192"
  }
]
'''

still a bit of an eyesore, but looks way better than that string escaping hell, lgtm

[jmank88]

still a bit of an eyesore, but looks way better than that string escaping hell, lgtm

This was a bit hacky. Working on a cleaner version.

[nolag]

Please just verify that it's going through by putting invalid config in the field names or something to see it fail. Last time, it turned out to be ignoring the config. I wish there was an easy way to verify it's using the chain reader and codec...

[reductionista]

Please just verify that it's going through by putting invalid config in the field names or something to see it fail. Last time, it turned out to be ignoring the config. I wish there was an easy way to verify it's using the chain reader and codec...

For ChainReader, could we fix that by adding an INFO log message to the OCR2 delegate that spawns the median plugin, or maybe the median plugin itself? At least one of those places, maybe both, should be able to check whether ChainReader got initialized or is nil. Although if its only available in the plugin itself, not sure where those logs will go to

[jmank88]

Please just verify that it's going through by putting invalid config in the field names or something to see it fail. Last time, it turned out to be ignoring the config. I wish there was an easy way to verify it's using the chain reader and codec...

For ChainReader, could we fix that by adding an INFO log message to the OCR2 delegate that spawns the median plugin, or maybe the median plugin itself? At least one of those places, maybe both, should be able to check whether ChainReader got initialized or is nil. Although if its only available in the plugin itself, not sure where those logs will go to

Does the chain reader report itself as a named service? If so, you could check the /health endpoint to see if it is running.
Edit: I might have to fix this first, but it should be unblocked this week and in by the end of the month 🤞 smartcontractkit/chainlink-common#206

[cl-sonarqube-production]

SonarQube Quality Gate

73.1% Coverage on New Code (is less than 75%)

See analysis details on SonarQube