Meridian-IE / impact-evaluator

Impact Evaluator smart contract
Other
4 stars 0 forks source link

Allow RPC clients to list measurements submitted for a given round #57

Closed bajtos closed 1 year ago

bajtos commented 1 year ago

I would like to list all measurement CIDs submitted for the given round index.

This is not possible with the current version, because roundIndex is not indexed.

ieContract.filters.MeasurementsAdded(null, roundIndex, null)

Error:

Error: cannot filter non-indexed parameters; must be null (argument="contract.roundIndex", value="30", code=INVALID_ARGUMENT, version=abi/5.7.0)

Proposal: rework the event definition as follows:

   event MeasurementsAdded(
        uint indexed roundIndex,
        string cid,
        address indexed sender
    );
bajtos commented 1 year ago

I think we will need this feature to be able to implement self-healing - see https://github.com/filecoin-station/spark-evaluate/issues/3

juliangruber commented 1 year ago

I would like to list all measurement CIDs submitted for the given round index.

For my understanding, why?

bajtos commented 1 year ago

I would like to list all measurement CIDs submitted for the given round index.

For my understanding, why?

(1) I noticed that in IE round 43, the evaluation service did not assign fraud assessment to all measurements; some of the measurements ended up with undefined.

To debug this issue, I need to find all measurement CIDs that we submitted for round 43 and thus used by our evaluation service.

My current workaround is to run flyctl logs in spark-evaluate for ~40 minutes to collect all logs for one round, hoping that this round will produce the faulty assessments too, and then grep trough these logs to extract lines in the format

PREPROCESS ROUND {index}: Added measurements from CID {bafy...cid}

Once we set up log monitoring, I'll be able to find these log lines for past rounds, but it's still a manual process that I'd like to automate.

(2) To self-heal the inner state of the evaluate service, I am imagining that at startup, we need to query the chain to find all measurements submitted for the current round before we have started so that we can fetch them for evaluation.

(3) At a high level, it's not clear to me what is the MeasurementsAdded good for when it's not indexed. What would we use these events for?

Once we index the round index, these events allow us to answer the question "what measurements were recorded and evaluated in round XYZ".

Besides the two use cases above, this can also enable Station operators to verify that their measurements were included in the evaluation. I didn't think this feature through; maybe it will not need to query the events by round index but by a time range instead, in which case it's not a good argument in this discussion.

juliangruber commented 1 year ago

(1) I noticed that in IE round 43, the evaluation service did not assign fraud assessment to all measurements; some of the measurements ended up with undefined.

Got it! Being able to debug this after the fact is valid. I wonder if we should add this to spark-publish instead, however since the on-chain cost of indexing another argument is very low we can also do it on the contract level - where it's more accessible in the first place.

(2) To self-heal the inner state of the evaluate service, I am imagining that at startup, we need to query the chain to find all measurements submitted for the current round before we have started so that we can fetch them for evaluation.

100% agree, we need that anyway, I was just concerned with what you needed it for now, when you asked "I would like to list all measurement CIDs submitted for the given round index.".

(3) At a high level, it's not clear to me what is the MeasurementsAdded good for when it's not indexed. What would we use these events for?

These events are read by the spark-evaluate service, without these it doesn't know about any measurements cids

bajtos commented 1 year ago

These events are read by the spark-evaluate service, without these it doesn't know about any measurements cids

Ah, of course! 🤦🏻