raps v2

[benisgold]

Fixes APP-####

What changed (plus any additional context for devs)

i recreated raps for the following reasons

• raps v1 does not support different sets of params for different actions within the same rap
• raps v1 requires params for a "swap" type action, limiting raps to those involving swaps
• code is hard to read bc of different types of raps that should be more isolated but instead are lumped together, causing messy types
• lots of type casting
• if i added the claimables rap to this, it would get much more messy bc it's pretty different from existing raps

Screen recordings / screenshots

What to test

nothing to test bc this isn't in use

[brunobar79]

Launch in simulator or device for 0d9dd0f

[brunobar79]

Launch in simulator or device for 40c448d

[brunobar79]

Launch in simulator or device for 8f92d46

[estebanmino]

is this the same than old raps? feels like a unnecessary code duplication

[estebanmino]

necessary comments?

[benisgold]

i uncomment in a followup pr that's currently open for review, i commented this code temporarily just so i could split my large branch into isolated PRs

[estebanmino]

this could just be

const claim = createNewAction('claimTransactionClaimableAction', parameters.claimTransactionClaimableActionParameters);
const newRap = createNewRap([claim]);

if I'm remembering correctly concat. actions was for raps with 2 or more actions

[benisgold]

yeah unnecessary for now but my next task after getting these merged is to add more actions to this rap so i think it's fine to keep it

[estebanmino]

i see a lot of this is a dupe from old raps, I understand there are types conflicts but there's no chance to avoid this?

[benisgold]

it's definitely avoidable, i felt that it was safer and more clear to keep v1 and v2 isolated. i can understand why that could be confusing when trying to tell which code is new and which is old.

[walmat]

I don't understand the need for this PR. We could just add the claimable action to the old raps and change the input parameters to be based on the new rap type.

[benisgold]

@walmat technically it isn't needed. i initially attempted type expansion, but it got messy pretty quickly. it was also difficult to follow my own code bc of all the numerous type assertions currently in raps. i figured i would try and clean things up

also for context the claimable rap types will be somewhat more complicated than what you see now because there will be the unlock/crosschain swap actions added onto this rap as well in future PRs

[benisgold]

@walmat if you want more context about my previous comment see my recent commit. i tested out (again) trying to make raps v1 compatible with claimables, and you can see how messy it gets with types. and this is just the tip of the iceberg, it gets worse. it's a slippery slope

[walmat]

@walmat if you want more context about my previous comment see my recent commit. i tested out (again) trying to make raps v1 compatible with claimables, and you can see how messy it gets with types. and this is just the tip of the iceberg, it gets worse. it's a slippery slope

i'll take a look

[derHowie]

Gunna ✅ this and do a deeper dive into the refactor itself once you migrate and the diff can be reasoned about. I think this means, though, that we should run smoke on the actions that use raps v1 prior to release.

[sentry-io]

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

• ‼️ Error: [rapsV2/execute]: claimTransactionClaimableAction - error execute action _callSuper(src/logger/index) View Issue

_{Did you find this useful? React with a 👍 or 👎}