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

Extended price messages #10

Merged
merged 14 commits into from
May 20, 2023
Merged

Extended price messages #10

merged 14 commits into from
May 20, 2023

Conversation

ckoopmann
Copy link
Collaborator

@ckoopmann ckoopmann commented May 19, 2023

Extends the message format that will be sent to the server by the following fields:

  1. interval_inclusion_messages: A list of signed message indicating all prices that are within a given interval
  2. value_message: A single message containing the "exact" price value
  3. validator_public_key: For signature validation on the server

The value_message and each element in interval_inclusion_messages is signed individually and includes the slot_number to avoid "replay attacks".

The values to which to attest in interval_inclusion_messages include all values within a relative range of +/- INTERVAL_SIZE_BASIS_POINTS and a precision of INTERVAL_STEP_DECIMALS.
So for current_price = 1800, INTERVAL_SIZE_BASIS_POINTS=100 (0.1%), INTERVAL_STEP_DECIMALS=2 (precision of 1 usd-cent) it would be ca. 1800 * 0.001 * 100 * 2 = 720 entries.

For the rust type definitions see here

Additional changes:

  1. Switch from the protocol_labs bls library to the one from lighthouse. (which I factored out into a separate repo to be able to install it without the rest of lighthouse) - for better serde deserialization and to avoid apparent issues with signature aggregation in the former
  2. Add a JsonFileMessageBroadcaster to write the messages to individual Json files in test_files for better inspection.

closes #3

@alextes
Copy link
Member

alextes commented May 19, 2023

I thought I left a comment about testing somewhere but can't find it. At the risk of repeating myself: I vaguely recall you mentioned you are all about testing, so where are all the tests 😄 ?

This is getting to a point where a bunch of testing seems healthy to me. Especially considering the fact we want this to be highly reliable. What is your thinking regarding testing?

@ckoopmann
Copy link
Collaborator Author

I thought I left a comment about testing somewhere but can't find it. At the risk of repeating myself: I vaguely recall you mentioned you are all about testing, so where are all the tests 😄 ?

This is getting to a point where a bunch of testing seems healthy to me. Especially considering the fact we want this to be highly reliable. What is your thinking regarding testing?

You are absolutely right, I should have done more testing.
I added some basic unit tests now for the components which are somewhat functional (price_provider, message_generator and signature_provider) as well as a very basic "integration test" where I mock out the gofer response with a static json file.

I didn't add unittests for components that are only placeholders / mocks and I suggest adding tests there when we replace those with the actual components. (message_broadcaster and slot_provider).

@ckoopmann ckoopmann merged commit 1ba2e9d into main May 20, 2023
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.

Finalize Message format and adjust implementation
2 participants