chore: review electra branch - part 1 #7015
Conversation
| @@ -407,16 +407,22 @@ export function getBeaconBlockApi({ | |||
|
|
|||
| async getBlockAttestations({blockId}) { | |||
| const {block, executionOptimistic, finalized} = await getBlockResponse(chain, blockId); | |||
| const fork = config.getForkName(block.message.slot); | |||
|
|
|||
| if (isForkPostElectra(fork)) { | |||
There was a problem hiding this comment.
We need to apply similar logic to other apis that only support phase0 once #6998 is merged
There was a problem hiding this comment.
After Electra is live, we should probably hardcode every deprecated attestation V1 endpoints to throw deprecation error ie. get rid of this if (isForkPostElectra(fork)) statement
There was a problem hiding this comment.
Yes, we can just statically throw an error and refer to v2 apis similar to this error. After electra is live we can also hard-code everywhere to use v2 apis only on the validator client side as all beacon node must have implemented them at this point, otherwise they can't handle electra attestations.
| @@ -59,7 +59,6 @@ | |||
| "types": "lib/index.d.ts", | |||
| "dependencies": { | |||
| "@chainsafe/as-sha256": "^0.5.0", | |||
| "@chainsafe/bls": "7.1.3", | |||
There was a problem hiding this comment.
Looks like it was added due to incorrect rebase, since #6894 we use blst directly in state-transition which seems questionable though, don't we want state-transition to be browser compatible?
There was a problem hiding this comment.
don't we want state-transition to be browser compatible?
not currently
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## electra-fork-rebasejul30 #7015 +/- ##
=========================================================
Coverage 49.37% 49.38%
=========================================================
Files 589 589
Lines 39137 39139 +2
Branches 2235 2231 -4
=========================================================
+ Hits 19325 19327 +2
Misses 19771 19771
Partials 41 41 |
| @@ -407,16 +407,22 @@ export function getBeaconBlockApi({ | |||
|
|
|||
| async getBlockAttestations({blockId}) { | |||
| const {block, executionOptimistic, finalized} = await getBlockResponse(chain, blockId); | |||
| const fork = config.getForkName(block.message.slot); | |||
|
|
|||
| if (isForkPostElectra(fork)) { | |||
There was a problem hiding this comment.
After Electra is live, we should probably hardcode every deprecated attestation V1 endpoints to throw deprecation error ie. get rid of this if (isForkPostElectra(fork)) statement
|
🎉 This PR is included in v1.22.0 🎉 |
No description provided.