Skip to content
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.

Sign up for GitHub

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

Clean Up Todos and Add in Issue Checker #640

Merged
merged 6 commits into from
Oct 11, 2018
Merged

Clean Up Todos and Add in Issue Checker #640

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a5945b9
Removing closed issues and adding issue checker to script
nisdas Oct 10, 2018
e1198a3
Merge branch 'master' of https://github.com/prysmaticlabs/geth-shardi…
nisdas Oct 10, 2018
286df8d
fix script
nisdas Oct 10, 2018
e09ad44
removing header
nisdas Oct 10, 2018
ae11371
add back randao issue
nisdas Oct 10, 2018
b00c389
Merge branch 'master' into issueChecker
rauljordan Oct 10, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions beacon-chain/blockchain/service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,6 @@ func NewChainService(ctx context.Context, cfg *Config) (*ChainService, error) {

// Start a blockchain service's main event loop.
func (c *ChainService) Start() {
// TODO(#474): Fetch the slot: (block, state) DAGs from persistent storage
Copy link
Member

Choose a reason for hiding this comment

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

Are we sure we don't need to do this anymore? @rauljordan can you confirm?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't believe we need this as we already do everything using persistent storage. This seems outdated.

// to truly continue across sessions.
log.Info("Starting service")

var err error
Expand Down
1 change: 0 additions & 1 deletion beacon-chain/db/schema.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ var (
simulatedBlockKey = []byte("last-simulated-block")

// Data item suffixes.
// TODO(#514): Change suffixes back to prefixes as originally designed after issue 514 is solved or after BoltDB migration
blockSuffix = []byte("-block") // blockhash + blockPrefix -> block
canonicalSuffix = []byte("-canonical") // num(uint64 big endian) + cannoicalSuffix -> blockhash
attestationSuffix = []byte("-attestation") // attestationHash + attestationSuffix -> attestation
Expand Down
2 changes: 0 additions & 2 deletions beacon-chain/types/crystallized_state.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,6 @@ func (c *CrystallizedState) isDynastyTransition(slotNumber uint64) bool {
func (c *CrystallizedState) getAttesterIndices(attestation *pb.AggregatedAttestation) ([]uint32, error) {
slotsStart := c.LastStateRecalculationSlot() - params.GetConfig().CycleLength
slotIndex := (attestation.Slot - slotsStart) % params.GetConfig().CycleLength
// TODO(#267): ShardAndCommitteesForSlots will return default value because the spec for dynasty transition is not finalized.
shardCommitteeArray := c.data.ShardAndCommitteesForSlots
shardCommittee := shardCommitteeArray[slotIndex].ArrayShardAndCommittee
for i := 0; i < len(shardCommittee); i++ {
Expand Down Expand Up @@ -312,7 +311,6 @@ func (c *CrystallizedState) NewStateRecalculations(aState *ActiveState, block *B
blockVoteBalance = 0
}

// TODO(#542): This should have been total balance of the validators in the slot committee.
if 3*blockVoteBalance >= 2*c.TotalDeposits() {
if slot > justifiedSlot {
justifiedSlot = slot
Expand Down
14 changes: 14 additions & 0 deletions scripts/check-todo.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,18 @@ then
echo "Invalid TODOs found. Failing." >&2;
echo "$OUTPUT" >&2;
exit 1;
fi


while read -r line ; do
linenum=`expr $line : '^\([0-9]*:\)'`
issueNum=${line//$linenum}
issueState=$(curl https://api.github.com/repos/prysmaticlabs/prysm/issues/$issueNum | grep -o '"state":"closed"');

if [ "$issueState" != "" ];
then
echo "Issue referenced has already been closed" >&2;
echo "Issue Number: $issueNum" >&2;
exit 1;
fi
done < <(grep -PrinH -o -h '(?<!context\.)todo\(#{0,1}\K(\d+)' --include \*.go *)
Copy link
Member

Choose a reason for hiding this comment

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

What is the purpose of this line? Wouldn't the script have exited on line 9 if there was any output of this command?

Copy link
Member Author

Choose a reason for hiding this comment

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

The do-while loop, reads any line printed out by grep, which parses the issue numbers from the files. Without that line the loop would not do anything as there is nothing printed out

Copy link
Member

Choose a reason for hiding this comment

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

Ah! Thanks!

1 change: 0 additions & 1 deletion shared/p2p/service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,6 @@ func (s *Server) Send(msg proto.Message, peer Peer) {

// Broadcast a message to the world.
func (s *Server) Broadcast(msg proto.Message) {
// TODO(#176): https://github.com/prysmaticlabs/prysm/issues/176
topic := s.topicMapping[messageType(msg)]
log.WithFields(logrus.Fields{
"topic": topic,
Expand Down
3 changes: 1 addition & 2 deletions validator/proposer/service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,8 +168,7 @@ func (p *Proposer) run(done <-chan struct{}, client pb.ProposerServiceClient) {
p.lock.Lock()

bitmask := p.GenerateBitmask(p.pendingAttestation)

// TODO(#552): Implement real proposals with randao reveals and attestation fields.
Copy link
Member

Choose a reason for hiding this comment

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

There is still a todo here, it's issue #619

Copy link
Member Author

Choose a reason for hiding this comment

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

The script basically just checks using the github api if the issue is closed. If it is closed it throws an error. I am just removing todos that are referencing issues that are closed

Copy link
Member

Choose a reason for hiding this comment

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

Np. I can add a TODO there later.

Copy link
Member Author

Choose a reason for hiding this comment

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

I added it back with #619.

// TODO(#619): Implement real proposals with randao reveals and attestation fields.
req := &pb.ProposeRequest{
ParentHash: latestBlockHash[:],
// TODO(#511): Fix to be the actual, timebased slot number instead.
Expand Down