Refactor BeaconStateSchema.getUniqueFields() methods

Consensys / teku

Open-source Ethereum consensus client written in Java

https://consensys.io/teku

Apache License 2.0

679 stars 284 forks source link

Refactor BeaconStateSchema.getUniqueFields() methods #8789

Closed jtraglia closed 3 weeks ago

jtraglia commented 3 weeks ago

PR Description

Instead of starting with Altair fields and re-adding everything since then, I think it would be better to inherit the fields from the previous upgrade then, if necessary, replace updated fields.

Documentation

[x] I thought about documentation and added the doc-change-required label to this PR if updates are required.

Changelog

[x] I thought about adding a changelog entry, and added one if I deemed necessary.

tbenr commented 3 weeks ago

Instead of starting with Altair fields and re-adding everything since then, I think it would be better to inherit the fields from the previous upgrade then, if necessary, replace updated fields.

Yes I agree with you. there has been a discussion in the past over this topic, I should go back and find my proposal. But essentially what we need is a way of overriding but also deprecating fields coming from the prior fork. I'll look into your approach probably on monday :)

tbenr commented 3 weeks ago

And thanks for your contribution, as usual!!

jtraglia commented 3 weeks ago

There has been a discussion in the past over this topic, I should go back and find my proposal.

Sounds good! If it's similar enough to your proposal, feel free to make whatever changes you want to this PR. Otherwise, I'm okay with closing this one and going with a different approach.