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.

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

RFC: bee-pow crate #19

Draft
wants to merge 8 commits into
base: master
Choose a base branch
from
Draft

RFC: bee-pow crate #19

wants to merge 8 commits into from

Conversation

Alex6323
Copy link
Contributor

@Alex6323 Alex6323 commented Oct 3, 2019

@Alex6323 Alex6323 changed the title Initial bee-pow RFC draft RFC: bee-pow Oct 3, 2019
@Alex6323 Alex6323 changed the title RFC: bee-pow RFC: bee-pow crate Oct 3, 2019
text/0000-bee-pow/0000-bee-pow.md Outdated Show resolved Hide resolved
tester: T,
}
impl<H, S, T, D, R> PearlDiver<H, S, T, D, R>
Copy link
Collaborator

@wusyong wusyong Nov 4, 2019

Choose a reason for hiding this comment

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

As long as I pretty love these trait layers, I feel like generics in PearlDiver is a little bit out of hand. Would it be better to use trait object in this case? Don't get me wrong, I actually prefer static dispatch over dynamic. Just wanna bring some suggestions and see if it fits in. I would like to hear others' opinion between trait bound and trait objects.

Suggested change
impl<H, S, T, D, R> PearlDiver<H, S, T, D, R>
struct PearlDiver<D, R> {
hasher: Box<dyn Hasher<Data = D, Hash = R>>,
sampler: Box<dyn Sampler<Data = D>>,
tester: Box<dyn Tester<Hash = R>>,
}

The `Sampler` trait allows implementing different types that mutate the input data handed to the hasher on each try. The standard sampler used in IOTA simply starts with a zero nonce and increments it until it finds a valid one. There are, however, other samplers possible, like a random sampler.

```Rust
pub trait Sampler {

Choose a reason for hiding this comment

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

Not sure where to put this comment, so it'll be here. The proposed design is not very well suited for data-parallel (using SIMD instructions) instances. At the moment fastest hashcash and pearldiver implementations use PCurl, which is "parallel" Curl capable of hashing n data/transactions (where n=64..256) at once. Of course, Sampler::Data can encapsulate such data-parallelism, but in my opinion it's not very convenient and it's worth baking it into the traits.

pub trait Sampler {
type Data;
fn next(&mut self, data: &mut Self::Data);

Choose a reason for hiding this comment

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

next should have bool as return type and return false in case the search data is exhausted. In real world cases we'll hardly exhaust it but it might be useful in tests and as additional guarantees that PoW doesn't loop infinitely.

Copy link
Contributor Author

@Alex6323 Alex6323 Nov 5, 2019

Choose a reason for hiding this comment

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

I agree with you.

```Rust
pub trait Sampler {
type Data;

Choose a reason for hiding this comment

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

Again, from my experience in hashcash and pearldiver the search data should be prepared in some non-trivial way (mostly, in order to guarantee that parallel ranges do not overlap). I would consider something like this:

    type Data;
    type SearchData;
    fn prepare(&mut self, data: &Self::Data, search_data: &mut Self::SearchData);
    fn next(&mut self, search_data: &mut Self::SearchData) -> bool;

Copy link
Contributor Author

@Alex6323 Alex6323 Nov 5, 2019

Choose a reason for hiding this comment

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

very good input, thanks. I will think about this more.

Am I correct in assuming that by "prepared in some non-trivial way" you mean conversion from t1b1 to ptrits (vectorizing) ?

whereby

```Rust
pub type PoWError = Box<dyn std::error::Error>;
Copy link
Contributor

@zesterer zesterer Nov 4, 2019

Choose a reason for hiding this comment

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

Is there any particular reason that PoWError must be dynamically-allocated?

Copy link
Contributor Author

@Alex6323 Alex6323 Nov 5, 2019

Choose a reason for hiding this comment

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

I think not. Any suggestion?

@Alex6323 Alex6323 force-pushed the rfc-pow branch 5 times, most recently from 344f731 to 60e695c Compare Dec 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Reviewing process
  
Backlog
Development

Successfully merging this pull request may close these issues.

None yet

5 participants