Skip to content

Conversation

@avilagaston9
Copy link
Contributor

@avilagaston9 avilagaston9 commented Dec 3, 2024

Telemetry: remove sensitive data exposure

Description

Changes the /versions endpoint to only expose the following fields of the operator:

  • operator_id
  • name
  • version
  • address
  • stake

How to Test

  1. Start anvil:
make anvil_start_with_block_time
  1. Start telemetry:
make telemetry_clean_db
make telemetry_full_start
  1. Start aggregator:
make aggregator_start
  1. Register and start an operator:
make operator_register_and_start
  1. Go to localhost:4001/versions and check that the displayed data is the expected.

  2. Also check the endpoint for that specific operator in localhost:4001/versions/{operator_address}.

  3. Finally, check that localhost:4001/api/operators and localhost:4001/api/operators/{operator_address} keeps displaying all operator data, not just the public.

Note

The version may appear with a null value. This is because the version variable in the operator is set in compile time and we are not running the operator compiled binary.

Type of change

  • Bug fix

Checklist

  • “Hotfix” to testnet, everything else to staging
  • Linked to Github Issue
  • This change depends on code or research by an external entity
    • Acknowledgements were updated to give credit
  • Unit tests added
  • This change requires new documentation.
    • Documentation has been added/updated.
  • This change is an Optimization
    • Benchmarks added/run
  • Has a known issue
  • If your PR changes the Operator compatibility (Ex: Upgrade prover versions)
    • This PR adds compatibility for operator for both versions and do not change batcher/docs/examples
    • This PR updates batcher and docs/examples to the newer version. This requires the operator are already updated to be compatible

@avilagaston9 avilagaston9 self-assigned this Dec 3, 2024
@avilagaston9 avilagaston9 marked this pull request as ready for review December 3, 2024 15:10
Copy link
Member

@MarcosNicolau MarcosNicolau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@uri-99 uri-99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm missing the information of version and rpc, but may be a local issue

@PatStiles
Copy link
Contributor

Ran locally and saw the described information.

@JulianVentura
Copy link
Contributor

I'm missing the information of version and rpc, but may be a local issue

This may happen if the operator is started before the telemetry service is on, since said information is provided at operator startup on SendTelemetryData function. Please, check if restarting the operator solves the problem

Copy link
Contributor

@JulianVentura JulianVentura left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good and it worked as expected locally

@JuArce JuArce merged commit 399c50c into testnet Dec 5, 2024
1 check passed
@JuArce JuArce deleted the fix-telemetry-show-only-public-data branch December 5, 2024 15:20
PatStiles pushed a commit that referenced this pull request Dec 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants