Right now for user actions if the OracleRecord is Some, we set OracleResponses to Replay, regardless of what the OracleRecord contains. On the system API calls that deal with Replay, the pattern we see is that if the OracleResponses is set to Replay, we'll try to replay the OracleResponses inside. If we have no OracleResponses inside, we'll error out with ExecutionError::MissingOracleResponse.
The pattern we see on tests right now is to pass Some(OracleRecord::default()) as the OracleRecord, which will generate a set of empty OracleResponses, which would fail. The reason it doesn't fail right now is that we don't test these system API calls using this pattern.
Proposal
All that is to say that I believe the pattern we should use is None as the OracleRecord, because we should always be "recording" in tests, because it'll likely be the first time we're running things. So I'm altering the code to follow that pattern instead, as I believe it's more correct.
There's a chance we're setting Replay/Record in a wrong way, but I'm not sure, so open to comments if that's the case.
Motivation
Right now for user actions if the
OracleRecord
isSome
, we setOracleResponses
toReplay
, regardless of what theOracleRecord
contains. On the system API calls that deal withReplay
, the pattern we see is that if theOracleResponses
is set toReplay
, we'll try to replay theOracleResponse
s inside. If we have noOracleResponse
s inside, we'll error out withExecutionError::MissingOracleResponse
. The pattern we see on tests right now is to passSome(OracleRecord::default())
as theOracleRecord
, which will generate a set of emptyOracleResponses
, which would fail. The reason it doesn't fail right now is that we don't test these system API calls using this pattern.Proposal
All that is to say that I believe the pattern we should use is
None
as theOracleRecord
, because we should always be "recording" in tests, because it'll likely be the first time we're running things. So I'm altering the code to follow that pattern instead, as I believe it's more correct. There's a chance we're settingReplay
/Record
in a wrong way, but I'm not sure, so open to comments if that's the case.Test Plan
CI