Skip to content

Add shard state transition function#1326

Merged
hwwhww merged 20 commits into
devfrom
vbuterin-patch-17
Aug 11, 2019
Merged

Add shard state transition function#1326
hwwhww merged 20 commits into
devfrom
vbuterin-patch-17

Conversation

@vbuterin

Copy link
Copy Markdown
Contributor

No description provided.

@hwwhww hwwhww added the phase 1 label Jul 30, 2019

@hwwhww hwwhww left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good job! 🎉

  • I'm writing some more tests for shard_state_transition with minimal PHASE_1_FORK_EPOCH: SHARD_SLOTS_PER_BEACON_SLOT * SLOTS_PER_EPOCH * EPOCHS_PER_SHARD_PERIOD * 2. Found some bugs and some pending questions in my comments.
  • It seems GENESIS_SHARD_SLOT can be removed now.

Comment thread specs/core/1_shard-data-chains.md Outdated
Comment thread specs/core/1_shard-data-chains.md Outdated
Comment thread specs/core/1_shard-data-chains.md Outdated
Comment thread specs/core/1_shard-data-chains.md Outdated
Comment thread specs/core/1_shard-data-chains.md
Comment thread specs/core/1_shard-data-chains.md
Comment thread specs/core/1_shard-data-chains.md Outdated
Comment thread specs/core/1_shard-data-chains.md Outdated
attestations += 1

for i in range(len(attester_committee), MAX_PERSISTENT_COMMITTEE_SIZE):
assert block.core.attester_bitfield[i] is False or block.core.attester_bitfield[i] == 0 # TODO: FIX Bitvector

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Somehow block.core.attester_bitfield[i] returns 0.

@protolambda is it the expected behavior in pyspec?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The elements of bitfields are typed as Bit. Bits are int subtypes, since bool can't be changed. It's a trade-off to make bitlist fully consistent with other typed lists. The int output can be cast to a bool. Or you change the other operand to Bit(False). Or, if it is not already working, we change the equality function to accept a bool type when comparing with a Bit.

vbuterin and others added 3 commits July 31, 2019 17:25
Co-Authored-By: Hsiao-Wei Wang <hwwang156@gmail.com>
Comment thread specs/core/1_shard-data-chains.md Outdated
hwwhww added 5 commits August 1, 2019 13:32
1. Clean up configurations
2. Add `HISTORY_ACCUMULATOR_VECTOR`
3. Add `validate_state_root` flag in `shard_state_transition` for testing
4. Rename `history_acc` to `history_accumulator`
Comment thread specs/core/1_shard-data-chains.md Outdated
if len(block.core.data) > SHARD_BLOCK_SIZE_TARGET:
state.basefee += Gwei(max(1, state.basefee * (len(block.core.data) - SHARD_BLOCK_SIZE_TARGET) // QUOTIENT))
elif len(block.core.data) < SHARD_BLOCK_SIZE_TARGET:
state.basefee -= Gwei(max(1, state.basefee * (len(block.core.data) - SHARD_BLOCK_SIZE_TARGET) // QUOTIENT))

@hwwhww hwwhww Aug 1, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Although you cap state.basefee to be at least 1 in the next line (state.basefee = Gwei(max(1, min(...) )), but at this line, it seems possible if state.basefee is 1, and then state.basefee -= Gwei(max(1, ...) -> state.basefee = 1 - 2 = -1? It is not allowable since it's an SSZ uint64 field.

Maybe use a temporary variable here?

    diff_amount = Gwei(max(1, state.basefee * (len(block.core.data) - SHARD_BLOCK_SIZE_TARGET) // QUOTIENT))
    if len(block.core.data) > SHARD_BLOCK_SIZE_TARGET:
        basefee_diff = diff_amount
    elif len(block.core.data) < SHARD_BLOCK_SIZE_TARGET:
        basefee_diff = - diff_amount
    else:
        basefee_diff = 0

    state.basefee = Gwei(max(
        1,
        min(
            EFFECTIVE_BALANCE_INCREMENT // EPOCHS_PER_SHARD_PERIOD // SHARD_SLOTS_PER_BEACON_SLOT * SLOTS_PER_EPOCH,
            state.basefee + basefee_diff,
        )
    ))

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think people will complain about even temporary variables that could be negative 😄

I don't know, I'm fine with any approach.

vbuterin and others added 2 commits August 2, 2019 09:40
Co-Authored-By: Hsiao-Wei Wang <hwwang156@gmail.com>
vbuterin and others added 2 commits August 5, 2019 14:37
Also adds `total_bytes` to state. The goal is to facilitate easier fraud proofs, so that one needs to simply check two adjacent headers in a crosslink and their respective bodies to verify a fraud proof.
Comment thread specs/core/1_shard-data-chains.md Outdated

@hwwhww hwwhww left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

As telegram discussion, it may need more iterations, but since it's blocking others, suggest merging it now.

@hwwhww hwwhww merged commit 13f6f18 into dev Aug 11, 2019
@djrtwo djrtwo deleted the vbuterin-patch-17 branch May 20, 2020 23:12
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.

3 participants