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

Fix fullnode.dappnode alias #1640

Merged
merged 11 commits into from
Oct 17, 2023
Merged

Fix fullnode.dappnode alias #1640

merged 11 commits into from
Oct 17, 2023

Conversation

dappnodedev
Copy link
Contributor

fullnode.dappnode domain should point to the mainnet execution selected by the user, if any

Also, each network (prater, gnosis and lukso) has its own fullnode domain, following this pattern: <network>.fullnode.dappnode

@github-actions
Copy link

github-actions bot commented Oct 11, 2023

@github-actions github-actions bot temporarily deployed to commit October 11, 2023 08:45 Inactive
@github-actions
Copy link

github-actions bot commented Oct 11, 2023

DAppNode bot has built and pinned the release to an IPFS node, for commit: 2b7e9e5

This is a development version and should only be installed for testing purposes, install link

/ipfs/QmVJqfDMR79LPAZQSxb9hbHubhCBKQypL4Y3KpPiu33vQA

(by dappnodebot/build-action)

@github-actions github-actions bot temporarily deployed to commit October 11, 2023 14:31 Inactive
@github-actions github-actions bot temporarily deployed to commit October 11, 2023 14:38 Inactive
@github-actions github-actions bot temporarily deployed to commit October 13, 2023 08:14 Inactive
@github-actions github-actions bot temporarily deployed to commit October 13, 2023 08:15 Inactive
@github-actions github-actions bot temporarily deployed to commit October 13, 2023 08:19 Inactive
@dappnodedev dappnodedev marked this pull request as ready for review October 13, 2023 08:20
@dappnodedev dappnodedev requested a review from a team as a code owner October 13, 2023 08:20
@github-actions github-actions bot temporarily deployed to commit October 13, 2023 10:56 Inactive
@dappnodedev dappnodedev force-pushed the dappnodedev/fix-fullnode branch 2 times, most recently from b2270e7 to f365d21 Compare October 13, 2023 11:03
@github-actions github-actions bot temporarily deployed to commit October 13, 2023 11:05 Inactive
@github-actions github-actions bot temporarily deployed to commit October 13, 2023 11:08 Inactive
removeAlias: boolean;
prevExecClientDnpName?: ExecutionClient<T>;
newExecClientDnpName?: ExecutionClient<T>;
network?: T;
Copy link
Contributor

Choose a reason for hiding this comment

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

why is network optional and default to mainnet? wouldnt it be better to make it explicitly clear what network we want and make it a mandatory parameter?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I chose to do it in this way because "mainnet" is the actual purpose of the fullnode functionality and before we did not consider fullnode domain in other networks so it should be the main one

Copy link
Member

Choose a reason for hiding this comment

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

If we were ever to move our repo to Gnosis Chain or to zkEVM - to save Gas on publishing new packages - , what would be the way to implement the network parameter and the defaults? I would consider this question to decide on the implementation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we do that, I am afraid the name of the domain is the least of our problems :/

@github-actions github-actions bot temporarily deployed to commit October 16, 2023 10:40 Inactive
@dappnodedev dappnodedev force-pushed the dappnodedev/fix-fullnode branch from 6856d4c to 1304eb9 Compare October 17, 2023 10:16
@github-actions github-actions bot temporarily deployed to commit October 17, 2023 10:20 Inactive
@github-actions github-actions bot temporarily deployed to commit October 17, 2023 10:22 Inactive
@dappnodedev dappnodedev force-pushed the dappnodedev/fix-fullnode branch from 1304eb9 to 2b7e9e5 Compare October 17, 2023 13:45
@github-actions github-actions bot temporarily deployed to commit October 17, 2023 13:50 Inactive
@dappnodedev dappnodedev merged commit 891ab95 into develop Oct 17, 2023
7 checks passed
@dappnodedev dappnodedev deleted the dappnodedev/fix-fullnode branch October 17, 2023 14:07
@dappnodedev
Copy link
Contributor Author

As a side note, <network>.fullnode.dappnode domain will start working by the time nsupdate module is removed, i.e. when the PR #1645 is merged

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.

Domain fullnode.dappnode does not resolve to the mainnet execution client
4 participants