Allow Spark to observe added/removed roots

[bajtos]

Hello from the Spark team 👋

I reviewed the docs and the PDPVerifier contract to understand how can Spark maintain a list of Root CIDs that are covered by PDP checks.

I found the following four events - I think they cover all state changes relevant to us 👍🏻

pdp/src/PDPVerifier.sol

Lines 31 to 34 in 19993af

	event ProofSetCreated(uint256 indexed setId);
	event ProofSetDeleted(uint256 indexed setId, uint256 deletedLeafCount);
	event RootsAdded(uint256 indexed firstAdded);
	event RootsRemoved(uint256[] indexed rootIds);

I can imagine how a PDP observer can use ProofSetCreated and ProofSetDeleted events to maintain a view of all active proof sets 👍🏻

However, I don't understand how we can maintain a view of all active roots.

Question 1: observing when roots are added to a root-set

pdp/contracts/src/PDPVerifier.sol

Lines 280 to 282 in 14fd1c9

	// Appends new roots to the collection managed by a proof set.
	// These roots won't be challenged until the next proving period.
	function addRoots(uint256 setId, RootData[] calldata rootData) public returns (uint256) {

Let's say somebody calls addRoots() with the following arguments:

setId=1
rootData=[ {cid: 'baf1', rawSize: 64}, {cid: 'bafy2', rawSize: 256 } ]

The event RootsAdded provides only firstAdded argument, which seems to be the rootId index into the three per-proof-set mappings:

pdp/src/PDPVerifier.sol

Lines 323 to 325 in 19993af

	rootCids[setId][rootId] = root;
	rootLeafCounts[setId][rootId] = leafCount;
	proofSetLeafCount[setId] += leafCount;

Using my example above, how can I reconstruct setId=1 and rootData=[ {cid: 'baf1', rawSize: 64}, {cid: 'bafy2', rawSize: 256 } ] from the firstAdded field reported by the RootsAdded event?

I think you need to add at least setId field to the event payload.

If we can assume that addRoots() will map individual roots to root ids that are in sequence (i.e. firstRoot, firstRoot+1, ... firstRoot+N-1), then the combination of setId and firstRoot allows the observer to iterate over all roots. The remaining question is how to know when to stop the iteration (what was the length of the rootData array).

If my reasoning is correct, then we need to change the event signature to the following one:

event RootsAdded(uint256 indexed setId, uint256 indexed firstAdded, uint256 addedCount);

While thinking about this more:

(1)
I don't understand why is firstAdded indexed - how do you envision the client queries that will use this index? It makes sense to me to look up events linked to a specific root set (via setId). I struggle to find a use case for looking up the events using the firstAdded value. (If I wanted to look up the event by root id, then how would I go from a particular root id to the "firstAdded" value emitted when the root was aded?)

Proposal:

event RootsAdded(uint256 indexed setId, uint256 firstAdded, uint256 addedCount);

(2)
I find the name "firstAdded" very confusing, it does not tell me how to interpret the value. Something like idOfTheFirstRootAdded is easier to understand, but also rather verbose.

Here is a suggestion that works well for me:

event RootsAdded(uint256 indexed setId, uint256 firstRootId, uint256 lastRootId);

Then I can run the following code to reconstruct the root CIDs & sizes that were added to the proof set:

for (let rootId = firstRootId; rootId <= lastRootId; rootId++) {
  const cid = rootCids[setId][rootId]
  const size = rootLeafCounts[setId][rootId] * LEAF_SIZE
  // add {rootId, cid, size} to our view of roots
}

Alternatively, can we model the RootsAdded event after RootsRemoved event that provides an array of root ids? I think that's the most elegant solution. (But I understand it could be more expensive on the gas fees.)

event RootsAdded(uint256 indexed setId, uint256[] rootIds);

Question 2: observing when roots are removed from a root-set

pdp/contracts/src/PDPVerifier.sol

Lines 329 to 331 in 14fd1c9

	// scheduleRemovals scheduels removal of a batch of roots from a proof set for the start of the next
	// proving period. It must be called by the proof set owner.
	function scheduleRemovals(uint256 setId, uint256[] calldata rootIds) public {

This is similar to the previous question.

First, the event RootsRemoved does not seem to be emitted at all. Huh?? There is only one source code line containing the string RootsRemoved:
https://github.com/search?q=repo%3AFilOzone%2Fpdp%20RootsRemoved&type=code

Let's say somebody calls scheduleRemovals() with the following arguments:

setId=1
rootIds = [10, 20]

How can a party observing PDP state learn:

• Which setId+rootId pairs were removed?
• When exactly are these roots considered as no longer covered by PDP proof? Is it when the removal is scheduled or when the roots are actually removed at the start of the next proving period?

[bajtos]

Question 3: linking new root-sets to their owners

I imagine we will need to filter which root-sets we watch for changes, e.g. to watch only root-sets created by Storacha.

A possible solution is to use the root-set owner address as the filter (e.g. watch only root-sets owned by one of Storacha wallet addresses).

We need two features to enable that:

1. When a new root-set is created, get the information about the owner.
2. When an owner is changed, get a notification (event) about that.

The first requirement can be met with the current PDPVerifier version by calling getProofSetOwner in the handler for the event ProofSetCreated.

It would be more ergonomic for the consumers if ProofSetCreated included the owner in the payload. IMO, it makes sense to index this field, so that consumers can filter ProofSetCreated events using the owner address.

event ProofSetCreated(address indexed owner, uint256 indexed setId);

I don't see how to meet the second requirement with the current PDPVerifier version. I propose adding a new event emitted by claimProofSetOwnership():

event ProofSetOwnerChanged(address indexed oldOwner, uint256 indexed setId, uint256 indexed newOwner);