Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

STF Vectors #28

Open
wants to merge 169 commits into
base: master
Choose a base branch
from
Open

STF Vectors #28

wants to merge 169 commits into from

Conversation

davxy
Copy link
Contributor

@davxy davxy commented Dec 7, 2024

Keeping up to date all the PRs has become overwhelming.

This PR superseeds the previous STF vectors PRs.

In the future, the other PRs may be reopened to merge individual subsystems' STFs separately. For now, I prefer to consolidate everything into a single PR.


Partially address #21

@celadari
Copy link

Thank you for the reply Davides !

  1. Should nu and xi be added to the state vector per the current GP?

I meant $\nu$ and $\xi$
image
image

because they are used in the STF process_
image

  1. No they don't; where did you read that need to be sorted?

My bad, lack of sleep made my interpret
image
as sorting them but it just means keep the same order as b

@dakk
Copy link

dakk commented Dec 11, 2024

I meant ν and ξ

the v is a theta

@celadari you are right, reports tests are missing some required state: xi, theta and also the ancestors set used in $(0.5.2 - 11.36)

@sourabhniyogi
Copy link

Small fix in the validator statistics: "gas" here should be "accumulate_gas" to match JSON representation of Work results in codec JSON here.

@celadari
Copy link

Small question regarding test vector 'tiny/no_assurances_with_stale_report-1.json' in assurances:

  • How can the post state have a null reported work report if input assurances is empty ?

@davxy
Copy link
Contributor Author

davxy commented Dec 17, 2024

Small question regarding test vector 'tiny/no_assurances_with_stale_report-1.json' in assurances:

* How can the post state have a null reported work report if input assurances is empty ?

In particular, the first assignment is stale (timed out) and is therefore removed from the assignment sequence.

This aligns with the active removal of timed-out reports, as proposed in the upcoming GP 0.5.3: gavofyork/graypaper#160

screenshot-2024-12-17-08-38-08

@celadari
Copy link

celadari commented Dec 18, 2024

Hello, I have a question regarding the codec test vectors, specifically the file assurances_extrinsic.

When decoding the file (and inspecting the .json), the 'bitfield' field appears to be only 1 byte, instead of 43 bytes (the expected size to encode a 341-length bitstring). Is this intentional? Based on the GP notation and "assurance test vectors", this field shouldn’t include a length prefix, as it represents the set B_s.

If that’s the case, shouldn’t we use a 43-byte bitstring field instead?

@davxy
Copy link
Contributor Author

davxy commented Dec 18, 2024

Hello, I have a question regarding the codec test vectors, specifically the file assurances_extrinsic.

When decoding the file (and inspecting the .json), the 'bitfield' field appears to be only 1 byte, instead of 43 bytes (the expected size to encode a 341-length bitstring). Is this intentional? Based on the GP notation and "assurance test vectors", this field shouldn’t include a length prefix, as it represents the set B_s.

If that’s the case, shouldn’t we use a 43-byte bitstring field instead?

Codec vectors are currently provided for the 'tiny' variant, which has 2 cores (this 1 byre for the bitstring). If you find them useful, I can also provide them for the 'full' variant.

@celadari
Copy link

Codec vectors are currently provided for the 'tiny' variant, which has 2 cores (this 1 byre for the bitstring). If you find them useful, I can also provide them for the 'full' variant.

Yeah, that would be great—thank you Davide !

For context: the bitstring field has a fixed length, so decoding the block requires parameterizing it for tiny and full test vectors. However, this adds cascading parameterization across the block and sub-blocks, making it much more complex.
We would rather keep it as the full size encoding

@jimjbrettj
Copy link

jimjbrettj commented Dec 19, 2024

Question regarding the post state for assurances full test vectors. Specifically for the some_assurances-1.json full test.

I am getting a few reports that I am calculating to become available that the test cases do not expect. For example, the report at core index 7 I have calculated to become available while the test does not.

Is there something happening here specifically preventing this report from being made available (I see it's not stale, but maybe I missed something else)? It might just be a miscalculation on my end, but I wanted to check and see if there was a case I was not accounting for (and that this is indeed the expected output) since my calculations work for every other test.

UPDATE: This indeed was just an error on my part. Tests all look good and am passing on my end. thank you!

@basedafdev
Copy link

basedafdev commented Dec 20, 2024

should be fixed 12505a1

Hey quick question regarding stats test vector:

  • Since the reporters set was already computed from guarantee STF, can't we assume that we have it as in input for statistics STF? To compute R from E_G requires \kappa & \lambada, which are outside the scope of this STF

@davxy
Copy link
Contributor Author

davxy commented Dec 22, 2024

should be fixed 12505a1

Hey quick question regarding stats test vector:

* Since the reporters set was already computed from guarantee STF, can't we assume that we have it as in input for statistics STF? To compute R from E_G requires \kappa & \lambada, which are outside the scope of this STF

I initially adopted the strategy you're suggesting. But I switched to the current strategy after #28 (comment)

Why you need lambda btw?

@basedafdev
Copy link

basedafdev commented Dec 22, 2024

Contributor

From what I understand to deduce report set R, we need to compute G*, which requires \lambda_prime, \kappa, & \eta

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants