tari-project / triptych

BSD 3-Clause "New" or "Revised" License
0 stars 3 forks source link

Relax `Arc` wrapping requirement #65

Closed AaronFeickert closed 2 months ago

AaronFeickert commented 7 months ago

Both Statement and Witness require that certain fields be wrapped in Arc, under the assumption that the caller is likely to reuse data like parameters and input sets that are expensive to clone.

This requirement has a bad code smell; it could be that the caller would be fine using a more efficient Rc instead, or that it does not intend to reuse these values and therefore has no need for such a wrapping.

It may be preferable to refactor so as to avoid this requirement and provide the caller more flexibility.

AaronFeickert commented 3 months ago

From some preliminary digging, it appears that Arc is likely performant enough that relaxing to Rc would be unnecessary. Further, switching to Rc means losing thread safety. That being said, the documentation cautions that no_std targets may not support Arc.

A solution could be to choose either Arc or Rc based on a feature flag, but I suspect this would get messy.

Another solution could be switching to references, but this could also get messy with lifetime management.