Skip to content

Move TBs to protocol/#118

Merged
vqhuy merged 1 commit into
masterfrom
tbout
Nov 11, 2016
Merged

Move TBs to protocol/#118
vqhuy merged 1 commit into
masterfrom
tbout

Conversation

@arlolra
Copy link
Copy Markdown
Member

@arlolra arlolra commented Nov 11, 2016

Guess this has something to do w/ #89

@vqhuy
Copy link
Copy Markdown
Member

vqhuy commented Nov 11, 2016

Yeah, I would do the same thing for #89. I like the pad.Sign and it LTGM!

Comment thread merkletree/pad.go Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you add a comment for this method? Its name isn't really clear to me.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah, I wasn't sure exactly where we landed on the discussion in #105 (comment)

There seems to have been an agreement on "latest" being the most recently signed epoch. Do we like "current" as the one we're presently accepting registrations for? Or, as I used, "next"?

I suppose this prefix needs to apply to the signature as well.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Or we can remove the prefix, just like pad.Set? Then we may need to rename Lookup to something like LookupInLatestEpoch.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Alright, I dropped the prefix.

@arlolra
Copy link
Copy Markdown
Member Author

arlolra commented Nov 11, 2016

@c633 If you're ok with this, feel bold about merging.

@vqhuy vqhuy merged commit fa21b15 into master Nov 11, 2016
@vqhuy
Copy link
Copy Markdown
Member

vqhuy commented Nov 11, 2016

Merged in fa21b15. Thanks @arlolra !

@arlolra arlolra deleted the tbout branch November 11, 2016 06:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants