Allow multiple redemptions per epoch

[asalzmann]

Context and purpose of the change

Currently, and address can only redeem once per epoch. This was simpler to implement, but multiple redemptions per epoch are required to add redemptions to autopilot.

Brief Changelog

• If a user has already redeemed, add the redemption amount to their UserRedemptionRecord (instead of creating a new one)
• If a user has already redeemed, don't append the UserRedemptionRecord ID to the HostZoneUnbonding

The rest of the logic should work without being changed, but please review this carefully and think about potential ways it could break.

TODOs

• Integration test
• Unittest
• Think deeply about protocol design

[sampocs]

lgtm!

waiting to approve until the unit test is in here

[riley-stride]

Looking good! Added one question. Waiting until integration and unit tests to approve.

[sampocs]

Just pushed up a unit test. I also tested manually and it worked as expected

I think adding this to our long standing integrations tests is actually not as straightforward as it seems. But imo, I think the unit test is sufficient coverage

[sampocs]

One other issue - the record is key'd by just stride address. So if someone redeems twice with the same stride address, but a different receiver address, the second address will get ignored

I think this leaves us with the original attack vector.

First solution that comes to mind is the change the URR key, but that will require a painful migration. Curious if you guy can think of a better solution

[shellvish]

Looks great to me! We should add integration tests on top of the unit test, then I'm all signed off 🙂

Happy to add the integration tests too if that would be helpful!

[github-actions]

This pull request has been automatically marked as stale because it has not had any recent activity. It will be closed if no further activity occurs. Thank you!

[asalzmann]

Just pushed up a unit test. I also tested manually and it worked as expected

@sampocs is this unittest sufficient, or should I add more?

One other issue - the record is key'd by just stride address. So if someone redeems twice with the same stride address, but a different receiver address, the second address will get ignored
I think this leaves us with the original attack vector.
First solution that comes to mind is the change the URR key, but that will require a painful migration. Curious if you guy can think of a better solution

I think where we landed was to replace the sender address with the receiver - @sampocs did you happen to take notes on that call?

[sampocs]

@asalzmann

is this unittest sufficient, or should I add more?

Imo, it's sufficient - was there something else you thought should be included though?

I think where we landed was to replace the sender address with the receiver - @sampocs did you happen to take notes on that call?

Yeah we landed on replacing sender with receiver. This is done in #1035

Notes from that call are at the bottom of the PFM + Autopilot doc and are in the upgrade channel.

[shellvish]

LGTM!