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

[ADP-3344] Add currentPParams to NetworkLayer. #4819

Merged
merged 5 commits into from
Oct 29, 2024

Conversation

HeinrichApfelmus
Copy link
Contributor

This pull request changes the NetworkLayer to retrieve era-indexed protocol parameters.

In particular, this pull request removes the field currentProtocolParametersInRecentEras from NetworkLayer and adds a field currentPParams instead.

This pull request also refactors the local state query for currentProtocolParameters to use a conversion on top of PParams era instead of trying to mingle conversion and query.

Issue Number

ADP-3344

@HeinrichApfelmus HeinrichApfelmus self-assigned this Oct 23, 2024
@HeinrichApfelmus HeinrichApfelmus force-pushed the HeinrichApfelmus/ADP-3344/network-pparams branch 8 times, most recently from 1a4d46b to de99cb8 Compare October 28, 2024 12:17
@HeinrichApfelmus HeinrichApfelmus marked this pull request as ready for review October 28, 2024 12:21
@HeinrichApfelmus HeinrichApfelmus force-pushed the HeinrichApfelmus/ADP-3344/network-pparams branch 2 times, most recently from ef53212 to 0522825 Compare October 28, 2024 13:26
Copy link
Member

@Anviking Anviking left a comment

Choose a reason for hiding this comment

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

lgtm 👍

lib/benchmarks/exe/api-bench.hs Show resolved Hide resolved
@@ -430,7 +429,7 @@ withNetworkLayer tr pipeliningStrategy np conn ver tol action = do

-- | Network parameters and protocol parameters for the node's current tip.
data NetworkParams = NetworkParams
{ protocolParams :: MaybeInRecentEra Write.PParams
{ protocolParams :: Read.EraValue Read.PParams
, protocolParamsLegacy :: ProtocolParameters
Copy link
Member

Choose a reason for hiding this comment

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

So now this protocolParamsLegacy could be dropped — if we wanted to

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. The question is whether this investment into the legacy wallet is worth the effort. At the moment, I'd like to freeze it in place, at least.

@HeinrichApfelmus HeinrichApfelmus added this pull request to the merge queue Oct 28, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 28, 2024
@HeinrichApfelmus HeinrichApfelmus added this pull request to the merge queue Oct 28, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 28, 2024
@HeinrichApfelmus HeinrichApfelmus force-pushed the HeinrichApfelmus/ADP-3344/network-pparams branch from 0522825 to e759988 Compare October 29, 2024 13:41
@HeinrichApfelmus HeinrichApfelmus force-pushed the HeinrichApfelmus/ADP-3344/network-pparams branch from e759988 to 3b58287 Compare October 29, 2024 13:58
@HeinrichApfelmus HeinrichApfelmus added this pull request to the merge queue Oct 29, 2024
Merged via the queue into master with commit 72280a2 Oct 29, 2024
25 checks passed
@HeinrichApfelmus HeinrichApfelmus deleted the HeinrichApfelmus/ADP-3344/network-pparams branch October 29, 2024 15:43
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.

2 participants