penumbra-zone / penumbra

Penumbra is a fully private proof-of-stake network and decentralized exchange for the Cosmos ecosystem.
https://penumbra.zone
Apache License 2.0
380 stars 295 forks source link

validate transmission keys are valid decaf377 encodings at transaction build time #3476

Open redshiftzero opened 11 months ago

redshiftzero commented 11 months ago

Pointed out by @hdevalence:

If someone creates an address with a transmission key that is not a valid decaf377 encoding, sending an output to that address will crash the transaction plan's build step — validation only happens at the key agreement step, and the code .expect()s it.

Outputs to invalid transmission keys won't end up on chain because you can't construct a valid output proof to an invalid transmission key as the AddressVar associated with a NoteVar requires it to be a valid decaf377 element. However, transaction building shouldn't crash if one is given an invalid address, so we could either:

  1. Decompress the transmission key in Address::from_components returning None if it's invalid. Clean but expensive.
  2. Add some one off validation in the Planner API. Concretely we could make Planner::output return Result<&mut self> as the swaps do, and then in that method check the transmission key is valid. It's a little messy though.
aubrika commented 11 months ago

Removing from next release in favor of already-selected to-do items

hdevalence commented 11 months ago

The reason we don't need to do this in the next release is that it's not a breaking change. We should be focusing on working through the list of breaking changes we already identified needing to make.