Add NodeMetadataPayload::derive_worker_address()

[fjarri]

• Makes NodeMetadatPayload.decentralized_identity_evidence a fixed size array (instead of a boxed slice)
• Adds a NodeMetadataPayload::derive_worker_address() method to derive the worker address from the evidence
• Makes all addresses passed as bytes in Python bindings fixed size arrays (so now their length will be checked by pyo3-generated wrapper code)
• Remove the unnecessary PyResult being returned from to_bytes() (it is infallible)
• NodeMetadataPayload.canonical_address and RevocationOrder.ursula_address were renamed to staker_address.

For reviewers:

• I mimicked the signed message format used in NuCypher code (eth_account.messages.encode_defunct()), which is marked as deprecated. Here's our chance to possibly change it to something more recent? (pinging @cygnusv)
• The Python signing code produces a signature Rust does not understand, because the recovery byte is 27/28 instead of 0/1. At which stage should we be fixing that? In NuCypher's TransactingPower.sign_message()?

[codecov-commenter]

Codecov Report

Merging #2 (bda67c8) into master (ca7a0d8) will not change coverage.
The diff coverage is 0.00%.

@@          Coverage Diff           @@
##           master      #2   +/-   ##
======================================
  Coverage    0.00%   0.00%           
======================================
  Files          12      12           
  Lines         369     416   +47     
======================================
- Misses        369     416   +47     

Impacted Files	Coverage Δ
nucypher-core/src/address.rs	0.00% <0.00%> (ø)
nucypher-core/src/arrays_as_bytes.rs	0.00% <0.00%> (ø)
nucypher-core/src/fleet_state.rs	0.00% <0.00%> (ø)
nucypher-core/src/node_metadata.rs	0.00% <0.00%> (ø)
nucypher-core/src/revocation_order.rs	0.00% <0.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ca7a0d8...bda67c8. Read the comment docs.

[piotr-roslaniec]

Should we include worker_address in the node metadata? This way we won't have to pass it externally.

I think this is the only place where we use it, so IMHO that would make sense. And I don't think there is a scenario where we need to worry about getting this information ourselves and not trusting whatever we have on our hands at the moment.

Should we allow a differentiation between different types of verification failures (payload signature fail, decentralized evidence fail, signature format fail etc)? Or perhaps split evidence verification into a separate method?

From the perspective of the library user more verbose error messages would be helpful (I imagine "signature format fail" would be useful for detecting incompatibility etc.). Splitting evidence verification doesn't help me with readability, personally.

The Python signing code produces a signature Rust does not understand, because the recovery byte is 27/28 instead of 0/1.

At which stage should we be fixing that? In NuCypher's TransactingPower.sign_message()?

Enforcing compatibility as early as in nucypher-core would be the most convenient but also the least flexible of approaches.

A random idea: What do you think about returning recovery bytes in addition to returning the signature, and making a decision based on the contents of those bytes in TransactingPower.sign_message()?

[piotr-roslaniec]

LGTM 🎉
I left a response to your questions. Please consider re-adding me to review if anything comes up.

[fjarri]

At which stage should we be fixing that? In NuCypher's TransactingPower.sign_message()?

Yes, for now I settled on that variant (see nucypher/nucypher#2850). The result still passes our verify_eip_191() function, so I think it is fine. The sign_transaction() still keeps the recovery byte with the chain ID since it's important for Eth clients.

A random idea: What do you think about returning recovery bytes in addition to returning the signature, and making a decision based on the contents of those bytes in TransactingPower.sign_message()?

I am not sure I follow, returning from where?

[piotr-roslaniec]

I am not sure I follow, returning from where?

From Rust side to Python side.