Remove E from validator_client types where it's not required#6727
Remove E from validator_client types where it's not required#6727jxs wants to merge 5 commits intosigp:unstablefrom
Conversation
jxs
force-pushed
the
remove-e-from-validator-client
branch
2 times, most recently
from
30c3a7c to
96dcd1d
Compare
| object, | ||
| Web3SignerObject::Deposit { .. } | Web3SignerObject::ValidatorRegistration(_) | ||
| ) && fork_info.is_some() | ||
| if matches!(message_type, MessageType::ValidatorRegistration) && fork_info.is_some() |
There was a problem hiding this comment.
object can never be a Web3SignerObject::Deposit as a SignableMessage doesn't convert to it, see line 202 above
There was a problem hiding this comment.
Feels kinda hard to keep track if SignableMessage starts converting to a Web3SignerObject::Deposit in the future.
I don't see the issue with keeping it as is, but what you said makes sense as well.
Although, i don't have enough context on the web3signer to know for sure
jxs
force-pushed
the
remove-e-from-validator-client
branch
8 times, most recently
from
6f0df3d to
b01c568
Compare
jxs
force-pushed
the
remove-e-from-validator-client
branch
5 times, most recently
from
b4920c5 to
08b3156
Compare
…rom-validator-client
jxs
force-pushed
the
remove-e-from-validator-client
branch
from
08b3156 to
a9cc2fb
Compare
pawanjay176
left a comment
pawanjay176
left a comment
There was a problem hiding this comment.
LGTM.
I'd recommend running this version of the VC on testnets before merging to make sure there isn't something that breaks unexpectedly
|
|
||
| [dependencies] | ||
| account_utils = { workspace = true } | ||
| beacon_node_fallback = { workspace = true } |
There was a problem hiding this comment.
Not really sure why we had to add dependencies here
There was a problem hiding this comment.
yup you are right, it was because I had remove doppelganger_service as a separate crate but I have since removed again, thanks Pawan!
| object, | ||
| Web3SignerObject::Deposit { .. } | Web3SignerObject::ValidatorRegistration(_) | ||
| ) && fork_info.is_some() | ||
| if matches!(message_type, MessageType::ValidatorRegistration) && fork_info.is_some() |
There was a problem hiding this comment.
Feels kinda hard to keep track if SignableMessage starts converting to a Web3SignerObject::Deposit in the future.
I don't see the issue with keeping it as is, but what you said makes sense as well.
Although, i don't have enough context on the web3signer to know for sure
|
Whats the state of this guy? We still doing this? |
Came here to ask the same thing :D I implemented #6705 with this PR in mind. Unfortunately, there I introduce a new generic parameter to the duties service: |
thanks for the ping. I think we should merge @dknopik's PR instead as it contains these changes. So going to close this one |
4 participants
and the
Ethspecparameters can be passed on function all time