Murisi/remove reverse conversions #4290
Conversation
c5bc0a6 to
ba4d2c7
Compare
ba4d2c7 to
b9f4594
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4290 +/- ##
==========================================
+ Coverage 74.03% 74.16% +0.12%
==========================================
Files 345 345
Lines 110047 110510 +463
==========================================
+ Hits 81476 81961 +485
+ Misses 28571 28549 -22 ☔ View full report in Codecov by Sentry. |
| @@ -2076,13 +2035,12 @@ mod test_shielded_wallet { | |||
| // to construct conversions properly, just like the protocol does, i.e. | |||
| // with conversions for non-consecutive epochs | |||
|
|
|||
| // Query the balance with conversions at epoch 4 | |||
| // Query the balance with conversions at epoch 5 | |||
There was a problem hiding this comment.
Probably this test is not needed anymore since it's testing the functionality that this pr removes?
There was a problem hiding this comment.
Thanks, you're right. But maybe this test can just be repurposed since the specific checks that it does are still useful in the absence of inverse conversions...
There was a problem hiding this comment.
Right but I think for this test to make sense we should shield at epoch 4 instead of 5. Right now we shield at 5 and query at 5 so we just get back the same amount. We should instead shield at 4, query at epoch 4 expecting the same amount and then query at epoch 5 expecting the rewards
Describe your changes
An alternative to #4288 where the ability to choose the epoch to which amounts are exchanged to is simply removed. This functionality became redundant once pinned transactions were removed in #3142 . More generally, this functionality is only needed for properly implementing functionality like account statements and pinned transactions where the amounts that were received need to be printed out exactly as they were in the past without any influence from conversions in the current tree.
Checklist before merging
breaking::labelsnamada-docsreponamada-indexerornamada-masp-indexer, a corresponding PR is opened in that repo