Readme could be clearer

[0xPhaze]

Things that were unclear/ambiguous to me during this assignment:

• The inclusion of the public key in the polynomial.

  and

  Check whether the address is included in the polynomial. To do this, you will have to make sure the evaluation of polynomial at address is 0.

  made it sound ambiguous whether it was the public key or the hash that was part of the polynomial. It's the hash.

• This can be done by hashing the input value and asserting the evaluation of the polynomial at that hashed value to be zero

  What is the "input value" here? It's the hash of the address..

• Check that the hash of the polynomial is the same as the public polynomial hash provided

  Does not mention which hashing algorithm and value was used. It's std::hash::pedersen(polynomial)[0]

• the evaluation of the polynomial at that hashed value to be zero

  Does not mention how the polynomial variable relates to the actual polynomial, i.e. that polynomial represents the coefficients: p[0] + p[1] * x + p[2] * x * x.

• For some reason running assert(std::hash::blake2s(signature) == nullifier); caused a panic for me, splitting it up in two lines worked

  let hash = std::hash::blake2s(signature);
  assert(hash == nullifier);
  

  This is resolved in newer versions (>= 0.10.0), but ecrecover module requires < 0.10.

• The README could have included a hint to use https://github.com/colinnielsen/ecrecover-noir ecrecover::secp256k1::PubKey::from_unified and ecrecover. std::ecdsa_secp256k1::verify_signature exists as well, but it doesn't use the unified pub key, and doesn't have an easy way of getting the hash/ethereum address.

• I still don't know what hashed_message actually corresponds to. I'm guessing this would have to be created in the ethereum contract (or be some kind of constant). It's also not specified here https://github.com/ZKCamp/stealthdrop-assignment/blob/b402eec08e59d68c9c85d3a39613f82e57441ae8/utils/populate.ts#L71C7-L71C7

I'll submit a PR if you want me to update these.

[shubham-kanodia]

@0xPhaze,

We wanted the students to look into how those inputs are formed rather than describing everything in the README, since it adds to their learning. This is something you would have seen in PetPark assignment as well where tests were descriptive of the problem statement. We have followed a similar approach by adding this note

• Note: The file being used to populate the inputs is also very useful for understanding how the inputs are being formed.

Things like what the roots of polynomial are, input value, hashing algorithm, what the polynomial looks like etc should be inferred from the populate.ts file.

• The README could have included a hint to use https://github.com/colinnielsen/ecrecover-noir ecrecover::secp256k1::PubKey::from_unified and ecrecover. std::ecdsa_secp256k1::verify_signature exists as well, but it doesn't use the unified pub key, and doesn't have an easy way of getting the hash/ethereum address.

Makes sense, can you include this in the PR

• I still don't know what hashed_message actually corresponds to. I'm guessing this would have to be created in the ethereum contract (or be some kind of constant). It's also not specified here https://github.com/ZKCamp/stealthdrop-assignment/blob/b402eec08e59d68c9c85d3a39613f82e57441ae8/utils/populate.ts#L71C7-L71C7

For this, can you add a value in populate.ts file and hash it in the file itself. In signature verification the data is always hashed before signing it. The data can be of arbitrary type. We were just hashing the string "sign this" using keccak hash. The message is not important as long as the user signs the hash, however, it will be good to have it in the script.

[0xPhaze]

Fair enough. Being a bit vague forced me to look into the docs and play around a bit. And I overlooked the hint to look at populate.ts and was only looking at the noir code.

Through the discussions on circle, I'd suggest to include

• disclaimers and really make clear that this is in no way a safe stealthdrop implementation due to signature malleability
• the polynomial commitment requires to be public and needs to be checked against on-chain, otherwise anyone can validate arbitrary polynomials
• since the polynomial is public, anyone can easily check whether a given address is part of the stealthdrop receivers (though they might not be able to tell which of the addresses is actually withdrawing)
• the implementation is susceptible to front-running, to mitigate this, we could include the withdrawer address as part of the signature message (even better to include the usual eip712 domain separator as well: contract address and chain id) or as part of the circuit using some trivial asserts

Also, any reason that the polynomial coefficients were chosen in such a way? We could have just directly used the addresses/hashes instead of using CANDIDATES + 1 and 0x1 as a constant to add:

fn evaluate_polynomial(polynomial: [Field; CANDIDATES], x: Field) -> Field {
  let mut out: Field = 1;

  for i in 0..CANDIDATES {
    out *= (x - polynomial[i]);
  }
  
  out
}

[shubham-kanodia]

• disclaimers and really make clear that this is in no way a safe stealthdrop implementation due to signature malleability

I have added a note in the README

• the polynomial commitment requires to be public and needs to be checked against on-chain, otherwise anyone can validate arbitrary polynomials

We have made the edit in the code provided. As mentioned earlier, we just missed making it public (it is mentioned in the readme that the commitment is public)

• since the polynomial is public, anyone can easily check whether a given address is part of the stealthdrop receivers (though they might not be able to tell which of the addresses is actually withdrawing)

As mentioned in the circle comment, this does not affect the functionality of stealthdrop. The idea is only to delink the whitelisted address (which is public) and the address making the transaction (withdrawing address). It must be clear from the description in README

• the implementation is susceptible to front-running, to mitigate this, we could include the withdrawer address as part of the signature message (even better to include the usual eip712 domain separator as well: contract address and chain id) or as part of the circuit using some trivial asserts

The front-running part is handled on the contract end, which is not a part of this assignment. The signed message could be anything and hence we have just given a placeholder hash

Since we couldn't cover a lot of things to structure this as an assignment, you can build a DApp using the same idea, similar to noir-voting. That could have a clear end to end flow and somethings like proper structure to message and front-running which we couldn't make part of this assignment. It would be a good resource to have and you can use this assignment as a starting point.