-
Notifications
You must be signed in to change notification settings - Fork 3
fix: operation and sanity spec tests v1.5.0 #75
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
base: main
Are you sure you want to change the base?
Changes from 58 commits
58d5129
1bd597a
0d59855
039886b
242b0c9
847d265
29848c5
d29c857
2be0eaa
a493a2c
991c0c8
11dc025
41dea08
99f7725
b36bd5c
e8733fb
ef300a6
1ceb59b
a19884c
7312938
443faa8
dc05a5e
3cd28bf
a67e15f
a65a7b0
76551f3
aa304e7
eb1b956
e096549
3f6c9a5
842c886
9bf59f3
4815ffe
81679a6
0c194f1
34a151d
a38cb71
0ad403b
8eb7545
f7e193c
7c03a03
d67614b
8fcc8eb
8e6ddfa
bb93b56
f0260ff
57a9fa7
0560409
91be677
ab4cc95
05c4fcd
5583474
b8c4dde
5a80728
df56c40
cd09f44
e7f50b4
2d24e30
90a86d0
7f4574c
c19a5e3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,10 +25,11 @@ const TIMELY_TARGET = 1 << c.TIMELY_TARGET_FLAG_INDEX; | |
| const TIMELY_HEAD = 1 << c.TIMELY_HEAD_FLAG_INDEX; | ||
| const SLOTS_PER_EPOCH_SQRT = std.math.sqrt(preset.SLOTS_PER_EPOCH); | ||
|
|
||
| /// AT = AttestationType | ||
| /// for phase0 it's `ssz.phase0.Attestation.Type` | ||
| /// for electra it's `ssz.electra.Attestation.Type` | ||
| pub fn processAttestationsAltair(allocator: Allocator, cached_state: *const CachedBeaconStateAllForks, comptime AT: type, attestations: []AT, verify_signature: bool) !void { | ||
| pub fn processAttestationsAltair(allocator: Allocator, cached_state: *const CachedBeaconStateAllForks, attestations: anytype, verify_signature: bool) !void { | ||
| // AT = AttestationType | ||
| // for phase0 it's `ssz.phase0.Attestation.Type` | ||
| // for electra it's `ssz.electra.Attestation.Type` | ||
| const AT = @typeInfo(@TypeOf(attestations)).pointer.child; | ||
| const state = cached_state.state; | ||
| const epoch_cache = cached_state.getEpochCache(); | ||
| const effective_balance_increments = epoch_cache.effective_balance_increment.get().items; | ||
|
|
@@ -44,19 +45,20 @@ pub fn processAttestationsAltair(allocator: Allocator, cached_state: *const Cach | |
| // let newSeenAttestersEffectiveBalance = 0; | ||
|
|
||
| var proposer_reward: u64 = 0; | ||
| for (attestations) |attestation| { | ||
| for (attestations) |*attestation| { | ||
| const data = attestation.data; | ||
| try validateAttestation(AT, cached_state, attestation); | ||
| try validateAttestation(cached_state, attestation); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same comment as the |
||
|
|
||
| // Retrieve the validator indices from the attestation participation bitfield | ||
| const attesting_indices = try if (AT == Phase0Attestation) epoch_cache.getAttestingIndicesPhase0(&attestation) else epoch_cache.getAttestingIndicesElectra(&attestation); | ||
| const attesting_indices = try if (AT == Phase0Attestation) epoch_cache.getAttestingIndicesPhase0(attestation) else epoch_cache.getAttestingIndicesElectra(attestation); | ||
| defer attesting_indices.deinit(); | ||
|
|
||
| // this check is done last because its the most expensive (if signature verification is toggled on) | ||
| // TODO: Why should we verify an indexed attestation that we just created? If it's just for the signature | ||
| // we can verify only that and nothing else. | ||
| if (verify_signature) { | ||
| const sig_set = try getAttestationWithIndicesSignatureSet(allocator, cached_state, &attestation.data, attestation.signature, attesting_indices.items); | ||
| defer allocator.free(sig_set.pubkeys); | ||
| if (!try verifyAggregatedSignatureSet(&sig_set)) { | ||
| return error.InvalidSignature; | ||
| } | ||
|
|
@@ -77,8 +79,8 @@ pub fn processAttestationsAltair(allocator: Allocator, cached_state: *const Cach | |
| // At epoch boundary, 100% of attestations belong to previous epoch | ||
| // so we want to update the participation flag tree in batch | ||
|
|
||
| // Note ParticipationFlags type uses option {setBitwiseOR: true}, .set() does a |= operation | ||
| epoch_participation[validator_index] = flags_attestation; | ||
| // no setBitwiseOR implemented in zig ssz, so we do it manually here | ||
| epoch_participation[validator_index] = flags_attestation | flags; | ||
|
|
||
| // Returns flags that are NOT set before (~ bitwise NOT) AND are set after | ||
| const flags_new_set = ~flags & flags_attestation; | ||
|
|
@@ -107,15 +109,13 @@ pub fn processAttestationsAltair(allocator: Allocator, cached_state: *const Cach | |
| } | ||
| } | ||
| } | ||
|
|
||
| // Do the discrete math inside the loop to ensure a deterministic result | ||
| const total_increments = total_balance_increments_with_weight; | ||
| const proposer_reward_numerator = total_increments * epoch_cache.base_reward_per_increment; | ||
| proposer_reward += @divFloor(proposer_reward_numerator, PROPOSER_REWARD_DOMINATOR); | ||
| } | ||
|
|
||
| increaseBalance(state, try epoch_cache.getBeaconProposer(state_slot), proposer_reward); | ||
| // Do the discrete math inside the loop to ensure a deterministic result | ||
| const total_increments = total_balance_increments_with_weight; | ||
| const proposer_reward_numerator = total_increments * epoch_cache.base_reward_per_increment; | ||
| proposer_reward += @divFloor(proposer_reward_numerator, PROPOSER_REWARD_DOMINATOR); | ||
| } | ||
| increaseBalance(state, try epoch_cache.getBeaconProposer(state_slot), proposer_reward); | ||
| } | ||
|
|
||
| pub fn getAttestationParticipationStatus(state: *const BeaconStateAllForks, data: ssz.phase0.AttestationData.Type, inclusion_delay: u64, current_epoch: Epoch, root_cache: *RootCache) !u8 { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,7 @@ const Allocator = std.mem.Allocator; | |
| const CachedBeaconStateAllForks = @import("../cache/state_cache.zig").CachedBeaconStateAllForks; | ||
| const BeaconStateAllForks = @import("../types/beacon_state.zig").BeaconStateAllForks; | ||
| const ssz = @import("consensus_types"); | ||
| const s = @import("ssz"); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think in near future we should consider what we want to do with namespaces, right now we're using Should we just use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Prefer to reserve |
||
| const preset = @import("preset").preset; | ||
| const ForkSeq = @import("config").ForkSeq; | ||
| const computeEpochAtSlot = @import("../utils/epoch.zig").computeEpochAtSlot; | ||
|
|
@@ -19,11 +20,21 @@ pub fn processAttestationPhase0(allocator: Allocator, cached_state: *CachedBeaco | |
| const slot = state.slot(); | ||
| const data = attestation.data; | ||
|
|
||
| try validateAttestation(*const Phase0Attestation, cached_state, attestation); | ||
| try validateAttestation(cached_state, attestation); | ||
|
|
||
| // should store a clone of aggregation_bits on Phase0 BeaconState to avoid double free error | ||
| var cloned_aggregation_bits: s.BitListType(preset.MAX_VALIDATORS_PER_COMMITTEE).Type = undefined; | ||
| try s.BitListType(preset.MAX_VALIDATORS_PER_COMMITTEE).clone(allocator, &attestation.aggregation_bits, &cloned_aggregation_bits); | ||
| var appended: bool = false; | ||
| errdefer { | ||
| if (!appended) { | ||
| cloned_aggregation_bits.deinit(allocator); | ||
| } | ||
| } | ||
|
Comment on lines
+26
to
+33
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Alternatively we can couple the deinit logic with the next 3 try's (which is up to where we're interested in deinit-ing), relying on a random |
||
|
|
||
| const pending_attestation = PendingAttestation{ | ||
| .data = data, | ||
| .aggregation_bits = attestation.aggregation_bits, | ||
| .aggregation_bits = cloned_aggregation_bits, | ||
| .inclusion_delay = slot - data.slot, | ||
| .proposer_index = try epoch_cache.getBeaconProposer(slot), | ||
| }; | ||
|
|
@@ -39,18 +50,25 @@ pub fn processAttestationPhase0(allocator: Allocator, cached_state: *CachedBeaco | |
| } | ||
| try state.previousEpochPendingAttestations().append(allocator, pending_attestation); | ||
| } | ||
| const indexed_attestation = try epoch_cache.getIndexedAttestation(.{ | ||
| .phase0 = attestation.*, | ||
| }); | ||
| appended = true; | ||
|
|
||
| _ = try isValidIndexedAttestation(ssz.phase0.IndexedAttestation.Type, cached_state, indexed_attestation.phase0, verify_signature); | ||
| var indexed_attestation: ssz.phase0.IndexedAttestation.Type = undefined; | ||
| try epoch_cache.computeIndexedAttestationPhase0(attestation, &indexed_attestation); | ||
| defer indexed_attestation.attesting_indices.deinit(allocator); | ||
|
|
||
| if (!try isValidIndexedAttestation(ssz.phase0.IndexedAttestation.Type, cached_state, &indexed_attestation, verify_signature)) { | ||
| return error.InvalidAttestationInvalidIndexedAttestation; | ||
| } | ||
| } | ||
|
|
||
| /// AT could be either Phase0Attestation or ElectraAttestation | ||
| pub fn validateAttestation(comptime AT: type, cached_state: *const CachedBeaconStateAllForks, attestation: AT) !void { | ||
| pub fn validateAttestation(cached_state: *const CachedBeaconStateAllForks, attestation: anytype) !void { | ||
| const T = @typeInfo(@TypeOf(attestation)).pointer.child; | ||
| std.debug.assert(T == Phase0Attestation or T == ElectraAttestation); | ||
| const is_electra = T == ElectraAttestation; | ||
| const epoch_cache = cached_state.getEpochCache(); | ||
| const state = cached_state.state; | ||
| const slot = state.slot(); | ||
| const state_slot = state.slot(); | ||
| const data = attestation.data; | ||
| const computed_epoch = computeEpochAtSlot(data.slot); | ||
| const committee_count = try epoch_cache.getCommitteeCountPerSlot(computed_epoch); | ||
|
|
@@ -64,12 +82,12 @@ pub fn validateAttestation(comptime AT: type, cached_state: *const CachedBeaconS | |
| } | ||
|
|
||
| // post deneb, the attestations are valid till end of next epoch | ||
| if (!(data.slot + preset.MIN_ATTESTATION_INCLUSION_DELAY <= slot and isTimelyTarget(state, slot - data.slot))) { | ||
| if (!(data.slot + preset.MIN_ATTESTATION_INCLUSION_DELAY <= state_slot and isTimelyTarget(state, state_slot - data.slot))) { | ||
| return error.InvalidAttestationSlotNotWithInInclusionWindow; | ||
| } | ||
|
|
||
| // same to fork >= ForkSeq.electra but more type safe | ||
| if (AT == ElectraAttestation) { | ||
| if (is_electra) { | ||
| if (data.index != 0) { | ||
| return error.InvalidAttestationNonZeroDataIndex; | ||
| } | ||
|
|
@@ -93,7 +111,7 @@ pub fn validateAttestation(comptime AT: type, cached_state: *const CachedBeaconS | |
| // instead of implementing/calling getBeaconCommittees(slot, committee_indices.items), we call getBeaconCommittee(slot, index) | ||
| var committee_offset: usize = 0; | ||
| for (committee_indices) |committee_index| { | ||
| const committee_validators = try epoch_cache.getBeaconCommittee(slot, committee_index); | ||
| const committee_validators = try epoch_cache.getBeaconCommittee(data.slot, committee_index); | ||
| if (committee_offset + committee_validators.len > aggregation_bits_array.len) { | ||
| return error.InvalidAttestationCommitteeAggregationBitsLengthTooShort; | ||
| } | ||
|
|
@@ -123,7 +141,7 @@ pub fn validateAttestation(comptime AT: type, cached_state: *const CachedBeaconS | |
| return error.InvalidAttestationInvalidCommitteeIndex; | ||
| } | ||
|
|
||
| const committee = try epoch_cache.getBeaconCommittee(slot, data.index); | ||
| const committee = try epoch_cache.getBeaconCommittee(data.slot, data.index); | ||
| if (attestation.aggregation_bits.bit_len != committee.len) { | ||
| return error.InvalidAttestationInvalidAggregationBitLen; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,7 +18,7 @@ pub fn processAttestations(allocator: Allocator, cached_state: *CachedBeaconStat | |
| .phase0 => |attestations_phase0| { | ||
| if (state.isPostAltair()) { | ||
| // altair to deneb | ||
| try processAttestationsAltair(allocator, cached_state, ssz.phase0.Attestation.Type, attestations_phase0.items, verify_signatures); | ||
| try processAttestationsAltair(allocator, cached_state, attestations_phase0.items, verify_signatures); | ||
| } else { | ||
| // phase0 | ||
| for (attestations_phase0.items) |attestation| { | ||
|
|
@@ -27,7 +27,7 @@ pub fn processAttestations(allocator: Allocator, cached_state: *CachedBeaconStat | |
| } | ||
| }, | ||
| .electra => |attestations_electra| { | ||
| try processAttestationsAltair(allocator, cached_state, ssz.electra.Attestation.Type, attestations_electra.items, verify_signatures); | ||
| try processAttestationsAltair(allocator, cached_state, attestations_electra.items, verify_signatures); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it's clearer and less prone to error if the reader of this code explicitly knows that we're passing in an attestation type of |
||
| }, | ||
| } | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: probably better do @divFloor here