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

getProtocolParams use CNCLI if ENABLE_KOIOS=N #1808

Merged
merged 3 commits into from
Aug 29, 2024

Conversation

TrevorBenson
Copy link
Collaborator

@TrevorBenson TrevorBenson commented Aug 18, 2024

Description

Disables KOIOS access from getProtocolParams if ENABLE_KOIOS is set to N.

Motivation and context

Found unexpected KOIOS API access when trying to completely disable KOIOS. I had set both ENABLE_KOIOS and USE_KOIOS_API to N, but still found cases where traffic was hitting KOIOS endpoints.

Which issue it fixes?

Closes #1807

How has this been tested?

From inside a container using bash -x and capturing stderr to stdout and confirming the curl to cli_protocol_params API route no longer occurs when cncli.sh leaderlog is executed. Also using tcpdump to just examine traffic destined for the IP address of guild.koios.rest.

@TrevorBenson TrevorBenson requested review from Scitz0 and rdlrt August 18, 2024 21:55
@TrevorBenson
Copy link
Collaborator Author

TrevorBenson commented Aug 18, 2024

@rdlrt @Scitz0
Should this PR include conditional logic to also check if USE_KOIOS_API is set to N and also fallback to cardano-cli for protocol params?

  • cncli.sh ignores ENABLE_KOIOS in favor of checking USE_KOIOS_API environment variables.
  • cntools.sh does not consider either environment variable.
    • I could not get cntools.sh to cause this behavior of calling koios API's via updateProtocolParams function after a couple of tries of jumping between menus in cntools, even though it appeared to be part of the main loop.

scripts/cnode-helper-scripts/env Outdated Show resolved Hide resolved
@rdlrt rdlrt force-pushed the alpha branch 3 times, most recently from 7366184 to 73bebe5 Compare August 26, 2024 07:33
@TrevorBenson TrevorBenson force-pushed the disable-koios-when-vars-are-set branch from 12b0fa2 to c75f582 Compare August 29, 2024 05:36
Copy link
Contributor

@Scitz0 Scitz0 left a comment

Choose a reason for hiding this comment

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

👍

@rdlrt rdlrt merged commit 4571d38 into alpha Aug 29, 2024
1 check passed
@rdlrt rdlrt deleted the disable-koios-when-vars-are-set branch August 29, 2024 06:57
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.

KOIOS API can still be queried when ENABLE_KOIS and USE_KOIOS_API are set to N
3 participants