WIP: Extended rules for improved handling of RADON errors in smart contracts

[guidiaz]

Abstract

• *::NoReveals are renamed to *::InsufficientQuorum.
• *::InsufficientConsensus are renamed to *::InsufficientMajority.
• Some legacy RadonErrors captions are pluralized.
• RadErrors other than InsufficientCommits, InsufficientMajority, InsufficientQuorum or UnhandledIntercept are to be reported into a small set of new RadonErrors values.
• Additional RadError cases are proposed.
• RadError cases are now convertible into explanatory RadonErrors subcodes.
• One single badly formated Retrieve script will suffice to report a RadonErrors::MalformedDataRequest as the Tally result.

Motivation and rationale

Refactoring of *::NoReveals

While the RadError::NoReveals error is generated on the default implementation of a node only if no reveal transactions were received at the deadline epoch on which a data request resolution expires, it would be quite reasonable for custom setups to refrain from evaluating consensus on the resolution of some data request if the number of received reveals at the deadline epoch was just certain percentage below the number of witnesses required by such data request. Therefore, refering *::InsufficientQuorum would be a more generic description that could be potentially reused on different node setups.

Refactoring of *::InsufficientConsensus

While there are three different conditions where the protocol determines a "lack of consensus" in the resolution of a data request at its Tally stage, the currently named as InsufficientConsensus actually refers to the fact that no majority of values of the same type were revealed. Naming InsufficientConsensus one out of three possible reasons for not reaching "consensus" turns out to be a vague description, if not rather misleading. Renaming *::InsufficientConsensus to *::InsufficientMajority states a self-explanatory and clear distinction among the other potential conditions provoking a lack of consensus in the resolution of a data request.

Pluralization of some legacy RadonErrors

There are some RadError cases that can only be thrown at the Retrieve stage of a data request and that may be eventually included within a Tally report, but only if a majority of the data sources turn up triggering the same RadError case.

The same happens with the RadError::MalformedReveal and also with the RadError::EncodeReveal introduced in WIP-0026, where a majority of failing revealers (i.e. witnessing nodes) must concur in order for these RadErrors to finalize into an errored result.

In coherence with the fact that one single failing entity (data sources during Retrieve stage, or witnessing nodes during Aggregate stage) may not be sufficient for trascending such error codes, their equivalent RadonErrors should better be pluralized to:

• RadonErrors::EncodeReveals
• RadonErrors::HttpErrors
• RadonErrors::InconsistentSources
• RadonErrors::MalformedReveals
• RadonErrors::RetrievalsTimeout

New reportable error codes

From all possible RadErrors that can be thrown through the resolution of a data request, only a few have actually transcended to the RadonReport<RadonTypes> that gets ultimately serialized into the Tally result of a data request.

Nevertheless, the kind of a vast majority of error cases have been downcasted to either RadonErrors::UnhandledIntercept (i.e. all of the unhandled RadErrors thrown during any of the Retrieve, Aggregate and Tally stages of data requests), RadonErrors::TallyExecution (i.e. the few handled RadErrors cases that can be potentially thrown during the Tally stage of data requests) or exceptionally to RadonErrors::Unknown (i.e. if at least one inconsistent source is detected during Retrieve stage).

While an error description has been stringified into the final RadonReport<RadonTypes> in the first two cases, this has also been proven not to be a very convenient means to infer the actual cause of failing data requests, not to say impractical and economically inviable from the perspective of a smart contract.

New sets of RadonErrors and RadErrors are proposed as to enable data requesters (including smart contracts) to easily abstract a few general failing reasons:

• *::CircumstantialFailure ⇒ A majority of data sources could either be temporarily unresponsive or failing to report the requested data.
• *::InconsistentSources ⇒ At least one data source is inconsistent when queried through multiple transports at once.
• *::MalformedDataRequest ⇒ Either one or more data sources were badly formated, or so were the Aggregate or the Tally scripts.
• *::MalformedResponses ⇒ Values returned from a majority of data sources did not match the expected schema.

Additional RadError cases

While RadError::HttpOther was thrown whenever there was an issue while composing or executing the HTTP request involved in the retrieval from a data source, the following fine-grained cases will enable the data requester to differentiate formerly unhandable situations:

• RadError::InvalidHttpBody
• RadError::InvalidHttpHeaders
• RadError::InvalidHttpResponse

All RadError cases convertable into explanatory RadonErrors subcodes

Even though RadErrors are to be downcasted into a small selection of first-order error codes, all RadError cases are still prone to be referred via a second-order subcode within the serialization of the resulting Tally report. In order to avoid losing track of the originating RadError, the conversion into a RadonErrors::UnhandledIntercept should never occur from now on, unless some error with no matching RadError case was actually produced during the resolution of a data request.

Also for the sake of conciseness, the downcasting of a RadError::Subscript error into a RadonErrors value should now be resolved recursively, based on the self-contained inner field (i.e. &Box<RadError>).

Forcing data requests with a minority of badly formated retrieval scripts to fail

There are a series of RadError cases that provide evidence of at least one Retrieve script being badly formated, but that have been silenced when triggered by just a minority of sources in multi-source data requests. While such partially malformed data requests can be early detected if dry run locally before being broadcasted to the Witnet blockchain, this trial workflow may just not be practical from the perspective of smart contracts that dynamically compose their own Witnet data requests.

Moreover, whereas some data requests may rely on multiple sources for "redundancy" sake (and so, it would not be a big deal if a minority of sources were badly formated), a vast majority of data requests (e.g. price feeds) do so for the sake of "data composability" (i.e. extracting or deriving an aggregated result from different providers). As to avoid this sort of data requests never coming to provide a complete version of the actual data being requested, a single badly formated Retrieve script must provoke a data request to resolve into a RadonErrors::MalformedDataRequest error code.

Smart contracts capable of dynamically building multi-source data requests will surely be capable of counter-acting should their data requests fail due to a single source resolving into a RadonErrors::MalformedDataRequest error code.

[drcpu-github]

Refactoring of *::NoReveals

While the RadError::NoReveals error is generated on the default implementation of a node only if no reveal transactions were received at the deadline epoch on which a data request resolution expires, it would be quite reasonable for custom setups to refrain from evaluating consensus on the resolution of some data request if the number of received reveals at the deadline epoch was just certain percentage below the number of witnesses required by such data request. Therefore, refering *::InsufficientQuorum would be a more generic description that could be potentially reused on different node setups.

I am somewhat confused what is being proposed here? It reads like you are implying it would be fine for a default node to include a tally with InsufficientQuorum when he sees zero reveals (and only then) while a non-default node would also be allowed to construct a tally with InsufficientQuorum on seeing more than zero, but still an insufficient amount of, reveals. This seems like invalid behavior in the sense that every miner constructs its tally locally and they would not agree on what the result would be?

While there are three different conditions where the protocol determines a "lack of consensus" in the resolution of a data request at its Tally stage, the currently named as InsufficientConsensus actually refers to the fact that no majority of values of the same type were revealed.

Could you detail what the other three conditions are and what the error would be in those (other) cases? Also, I was under the impression that the current InsufficientConsensus result was value-based, not necessarily type-based?

Nevertheless, the kind of a vast majority of error cases have been downcasted to either RadonErrors::UnhandledIntercept

Yeah, this would be very nice to improve upon!

[guidiaz]

I am somewhat confused what is being proposed here? It reads like you are implying it would be fine for a default node to include a tally with InsufficientQuorum when he sees zero reveals (and only then) while a non-default node would also be allowed to construct a tally with InsufficientQuorum on seeing more than zero, but still an insufficient amount of, reveals. This seems like invalid behavior in the sense that every miner constructs its tally locally and they would not agree on what the result would be?

This idea was proposed under the (wrong) assumption that a DR Tally could be potentially processed even though not all the DR Reveal transactions got mined before the DR resolution deadline. It seems though that there's currently a block validation condition that prevents this from ever happening. If so, I admit that renaming NoReveals to InsufficientQuorum would not make too much sense, and indeed could also be interpreted as implying a change on block validation rules, which is not the intention at all in this WIP.

But if indeed, could a limited number DR Reveal transactions get included in blocks prior to the DR deadline, but not just enough to fulfill the DR requirement, I believe that renaming NoReveals to InsufficientReveals would definitely make sense.

[drcpu-github]

This idea was proposed under the (wrong) assumption that a DR Tally could be potentially processed even though not all the DR Reveal transactions got mined before the DR resolution deadline. It seems though that there's currently a block validation condition that prevents this from ever happening. If so, I admit that renaming NoReveals to InsufficientQuorum would not make too much sense, and indeed could also be interpreted as implying a change on block validation rules, which is not the intention at all in this WIP.

Yeah, I think the main reason for that is that it would open the door to abuse where you censor and slash nodes by publishing a tally too soon (e.g., publish an 8/10 reveal tally at the first reveal attempt, thereby slashing two nodes for not revealing).

[guidiaz]

Another approach would be to force all reveal transaction be mined altogether within the same block, just the same as it's already done with commit transactions, so in the end it would always be three, and only three, different mining nodes participating in the resolution of a data request: the one that inserts the DRT itself, the one that inserts all commits and the one that inserts all reveals and the tally (see #2059). This idea would be under the scope of a different RFC/WIP, ofc.

[guidiaz]

Could you detail what the other three conditions are and what the error would be in those (other) cases? Also, I was under the impression that the current InsufficientConsensus result was value-based, not necessarily type-based?

Atm, NoReveals, InsufficientCommits and InsufficientConsensus are the conditions upon which it won't be possible to resolve a DR due to some lack of participation/consensus at the Witnet protocol level: not enough commit transactions, not enough reveal transactions, or no majority of "congruent" values were revealed (respectively).