Changed the note selection algorithm to try and reduce the notes used.

[murisi]

Describe your changes

An attempt to optimize the MASP note selection algorithm in order to reduce the size of MASP transactions. Smaller transactions are desirable for reducing gas fees and ensuring that the hardware wallet is able to display transactions without erroring out. To this end, the following changes have been made:

• Refactored the note selection code so that it's easier to swap out the note selection algorithm
• Implemented a greedy note selection algorithm based on the approximation for set covers
• Implemented a Fraction structure to facilitate comparing notes to potentially be used in a transaction
• Combined the creation of conversion descriptions to potentially further reduce transaction size

The major drawback of the new note selection algorithm is that it exchanges all unspent notes first before attempting to choose the most optimal notes. This makes it easier to handle situations where users want to spend NAM and this NAM has to be collected from the rewards because they don't have enough NAM notes. In this case the algorithm should select non-NAM notes that yield the largest NAM rewards after being exchanged.

Another drawback of the new algorithm is that it doesn't account for the space/bytes used by the conversions that accompany a note. Probably notes that require a lot of conversions should be less likely to be picked, i.e. they should have some sort of cost. This hasn't been investigated because it's not yet clear whether this will be a problem in practice. If it does become a problem, it might be possible to use a weighted version of the set cover approximation algorithm.

Perhaps the proposed algorithm is not the best, but hopefully it's now a bit easier to implement a more optimal note selection algorithm.

This PR depends upon namada-net/masp#100 for the dot product, supremum, and infimum operations used when manipulating note contributions.

Examples

What follows are printouts of the note selection algorithms' executions. The target amount is the minimum amount of value that we are trying to collect. Notes are first printed out in the order in which they are encountered. And then they are printed out in the order in which they are selected for the transaction. The ideal is to select the most effective notes first and the least effective ones last if they are still needed.

1) Before

Target: ValueSum({(Zero, Established: tnam1q9kn74xfzytqkqyycfrhycr8ajam8ny935cge0z5): 190982})
Encountered: ValueSum({(Zero, Established: tnam1q9kn74xfzytqkqyycfrhycr8ajam8ny935cge0z5): 95088, (Zero, Established: tnam1qy88jaykzw8tay6svmu6kkxxj5xd53w6qvqkw20u): 10000000})
Encountered: ValueSum({(Zero, Established: tnam1q9kn74xfzytqkqyycfrhycr8ajam8ny935cge0z5): 95894})

Using: ValueSum({(Zero, Established: tnam1q9kn74xfzytqkqyycfrhycr8ajam8ny935cge0z5): 95088, (Zero, Established: tnam1qy88jaykzw8tay6svmu6kkxxj5xd53w6qvqkw20u): 10000000})
Using: ValueSum({(Zero, Established: tnam1q9kn74xfzytqkqyycfrhycr8ajam8ny935cge0z5): 95894})

1) After

Target: ValueSum({(Zero, Established: tnam1q9kn74xfzytqkqyycfrhycr8ajam8ny935cge0z5): 190982})
Encountered: ValueSum({(Zero, Established: tnam1q9kn74xfzytqkqyycfrhycr8ajam8ny935cge0z5): 95088, (Zero, Established: tnam1qy88jaykzw8tay6svmu6kkxxj5xd53w6qvqkw20u): 10000000})
Encountered: ValueSum({(Zero, Established: tnam1q9kn74xfzytqkqyycfrhycr8ajam8ny935cge0z5): 95894})

Using: ValueSum({(Zero, Established: tnam1q9kn74xfzytqkqyycfrhycr8ajam8ny935cge0z5): 95894})
Using: ValueSum({(Zero, Established: tnam1q9kn74xfzytqkqyycfrhycr8ajam8ny935cge0z5): 95088, (Zero, Established: tnam1qy88jaykzw8tay6svmu6kkxxj5xd53w6qvqkw20u): 10000000})

2) Before

Target: ValueSum({(Zero, Established: tnam1q9kn74xfzytqkqyycfrhycr8ajam8ny935cge0z5): 318510})
Encountered: ValueSum({(Zero, Established: tnam1q9kn74xfzytqkqyycfrhycr8ajam8ny935cge0z5): 126780, (Zero, Established: tnam1qy88jaykzw8tay6svmu6kkxxj5xd53w6qvqkw20u): 10000000})
Encountered: ValueSum({(Zero, Established: tnam1q9kn74xfzytqkqyycfrhycr8ajam8ny935cge0z5): 191730})

Using: ValueSum({(Zero, Established: tnam1q9kn74xfzytqkqyycfrhycr8ajam8ny935cge0z5): 126780, (Zero, Established: tnam1qy88jaykzw8tay6svmu6kkxxj5xd53w6qvqkw20u): 10000000})
Using: ValueSum({(Zero, Established: tnam1q9kn74xfzytqkqyycfrhycr8ajam8ny935cge0z5): 191730})

2) After

Target: ValueSum({(Zero, Established: tnam1q9kn74xfzytqkqyycfrhycr8ajam8ny935cge0z5): 318510})
Encountered: ValueSum({(Zero, Established: tnam1q9kn74xfzytqkqyycfrhycr8ajam8ny935cge0z5): 126780, (Zero, Established: tnam1qy88jaykzw8tay6svmu6kkxxj5xd53w6qvqkw20u): 10000000})
Encountered: ValueSum({(Zero, Established: tnam1q9kn74xfzytqkqyycfrhycr8ajam8ny935cge0z5): 191730})

Using: ValueSum({(Zero, Established: tnam1q9kn74xfzytqkqyycfrhycr8ajam8ny935cge0z5): 191730})
Using: ValueSum({(Zero, Established: tnam1q9kn74xfzytqkqyycfrhycr8ajam8ny935cge0z5): 126780, (Zero, Established: tnam1qy88jaykzw8tay6svmu6kkxxj5xd53w6qvqkw20u): 10000000})

Checklist before merging

• If this PR has some consensus breaking changes, I added the corresponding breaking:: labels
  • This will require 2 reviewers to approve the changes
• If this PR requires changes to the docs or specs, a corresponding PR is opened in the namada-docs repo
  • Relevant PR if applies:
• If this PR affects services such as namada-indexer or namada-masp-indexer, a corresponding PR is opened in that repo
  • Relevant PR if applies:

[grarco]

Thanks for this PR @murisi, I'm approving it but I would like to wait for the other reviewers before proceeding with it

[murisi]

Thanks for this PR @murisi, I'm approving it but I would like to wait for the other reviewers before proceeding with it

Thanks for reviewing @grarco , understood. When rereading this PR, I realized that the fraction code could be replaced with a nested loop to achieve the same result. Please let me know if the latest commit (671d0b5) is problematic... Thanks!

[brentstone]

This seems important and time-sensitive. Would it make sense to merge this asap and cut a patch release? It just involves client changes, correct?

@murisi @tzemanovic @sug0 @batconjurer

[murisi]

This seems important and time-sensitive. Would it make sense to merge this asap and cut a patch release? It just involves client changes, correct?

@murisi @tzemanovic @sug0 @batconjurer

Yes, this PR only involves client changes. However it depends upon namada-net/masp#100 being merged to main (and being published to crates.io). I'll merge MASP crate update just now...

[sug0]

@murisi would just fix that comment I left, otherwise lgtm

[tzemanovic]

replaced by #4763