Conversation
diegonehab
force-pushed
the
feature/output-unified-server-manager
branch
from
22bc6f7 to
3da49c0
Compare
server-manager.proto
Outdated
| OutputValidityProof validity = 4; // Validity proof for an output | ||
| bytes context = 5; // Data that allows the validity proof to be contextualized within submitted claims. Currently, the context is the epoch number as a ABI-encoded uint256. | ||
| OutputValidityProof validity = 3; // Validity proof for an output | ||
| bytes context = 4; // Data that allows the validity proof to be contextualized within submitted claims. Currently, the context is the epoch number as a ABI-encoded uint256. |
There was a problem hiding this comment.
Just an observation, for rollups-contracts v2, we don't need this context field anymore. Now, in order to retrieve the correct claim from the consensus, you only need to provide the application contract address (like before), and the input range (first input index and last input index). So, maybe we can change this field here to be that? We can discuss this further in the Output Unification meeting too. :-)
There was a problem hiding this comment.
Hmm. The input range is easy. I am sure we have this information lying arround. But would the Server Manager be informed, during session startup, of this contract address? And should we bundle it in the proof structure itself?
There was a problem hiding this comment.
I think the node already has hold of the application address, since it needs to filter out InputAdded events for a specific application address.
There was a problem hiding this comment.
Please correct me if I'm wrong, @gligneul.
There was a problem hiding this comment.
Yes, the node has the app address. It is reasonable for the node to append the app address to the Proof, so we don't need to pass it to the server manager.
On the other hand, It would be good if the server manager added the input range to the proof.
There was a problem hiding this comment.
@gligneul I think the InputRange struct can be on the FinishEpochResponse itself, since it would be the same for every output in the same epoch, right?
There was a problem hiding this comment.
How would the front end get the input range if we do that? There is no epoch structure in the GraphQL API.
There was a problem hiding this comment.
We already include stuff that would be the same for every output. I guess the idea was to give exactly what the user would need. Rather than have the user fish around for the required data. Has this understanding been changed?
There was a problem hiding this comment.
Has this understanding been changed?
No, it has not. Let's discuss this in more detail in the workgroup meeting.
There was a problem hiding this comment.
Alright, adding this to our agenda. 📓
diegonehab
force-pushed
the
feature/output-unified-server-manager
branch
3 times, most recently
from
0c43970 to
7a5449d
Compare
| OutputValidityProof validity = 4; // Validity proof for an output | ||
| bytes context = 5; // Data that allows the validity proof to be contextualized within submitted claims. Currently, the context is the epoch number as a ABI-encoded uint256. | ||
| OutputValidityProof validity = 3; // Validity proof for an output | ||
| InputRange input_range = 4; // Range of processed in the epoch |
There was a problem hiding this comment.
I think a noun is missing after "processed".
| OutputEnum output_enum = 3; // Type of the output | ||
| OutputValidityProof validity = 4; // Validity proof for an output | ||
| bytes context = 5; // Data that allows the validity proof to be contextualized within submitted claims. Currently, the context is the epoch number as a ABI-encoded uint256. | ||
| OutputValidityProof validity = 3; // Validity proof for an output |
There was a problem hiding this comment.
This is a proof for a specific output that has been mentioned above, so "the" output.
diegonehab
force-pushed
the
feature/output-unified-server-manager
branch
from
7a5449d to
5c72c42
Compare
| bytes exception_data = 4; // Exception payload when there was an EXCEPTION | ||
| } | ||
| repeated Report reports = 5; // Reports produced during input or query processing | ||
| repeated Output reports = 5; // Reports produced during input or query processing |
There was a problem hiding this comment.
Aren't these still reports?
Did we completely remove them?
4 participants
This PR updates the interface of the Server Manager for output unification.