Allow Address network prefix to be overriden for printing

[austinabell]

Summary of changes
Changes introduced in this pull request:

• Allows ability to set Address network specifically (Can be chained to use when specifically printing)
  • Was going to leave it to some configuration within Forest, but I can see the need for it to be specified explicitly (as it was by someone using this crate)
• Removed unnecessary heap allocation in tests I noticed
• Updated function signature for generating secp and actor addresses to only require reference to bytes since address is generated from hashing bytes and does not require the move

I tried to find the naming guideline for this kind of functionality but this makes sense to me, does with_network make sense to you?

Reference issue to close (if applicable)

Closes

Other information and links

[ansermino]

I'm not sure this functionality is the best course of action. In most protocols we associate network IDs with addresses to prevent replay attacks, what the exact purpose is in FIL?

My thought is that this functionality should only exist as a constructor, not as a method to modify (a copy of) an existing address instance. We should not be associating different network IDs with existing address instances, rather only creating instances with the current ID and discarding any we receive that don't match that ID.

[austinabell]

I'm not sure this functionality is the best course of action. In most protocols we associate network IDs with addresses to prevent replay attacks, what the exact purpose is in FIL?

My thought is that this functionality should only exist as a constructor, not as a method to modify (a copy of) an existing address instance. We should not be associating different network IDs with existing address instances, rather only creating instances with the current ID and discarding any we receive that don't match that ID.

I would agree, but defined in the protocol the network prefix is not serialized to bytes, so it's not used in any consensus but just for printing. When deserializing bytes on another network, we may have a way to pass through the config to set the network but for some cases we will need the ability to modify the network of an Address.

But aside from that this couldn't introduce any vulnerabilities, someone with external code could still swap out the network prefix without this change, no external code could affect how our client operates, I fail to see how this is a concern.

Edit: The use case is that if you deserialize from bytes, there is no way to set what the network actually is, because there is no data indicating this, this is the primary reason for the change

[dutterbutter]

As per your question, I think it makes more sense to name this set_network given its functionality. Just my opinion tho

[austinabell]

ya, I didn't like that the name didn't indicate self is mutated, with set_network it doesn't indicate that it can be chained and returns a mutable self ref. I'll think about this later and see if any examples in std lib for naming

[ec2]

with kinda feels like its a temporary change. set feels more permanent

[austinabell]

yeah I agree this is better, I didn't like that it didn't indicate that it could be chained, but that is definitely less of a problem than it being worded as not being mutated and instead being copied