feat: top level withdrawal methods [OTE-389] [1/2] #417
Conversation
yogurtandjam
force-pushed
the
jerms/OTE-388_top-level-deposit-methods
branch
from
34c288a to
a3a42da
Compare
yogurtandjam
force-pushed
the
jerms/OTE-389_top-level-withdrawal-methods
branch
from
1ab2d4b to
96f3d50
Compare
yogurtandjam
force-pushed
the
jerms/OTE-389_top-level-withdrawal-methods
branch
from
27617af to
9a96fbf
Compare
yogurtandjam
force-pushed
the
jerms/OTE-389_top-level-withdrawal-methods
branch
from
13f0b0e to
b62e37b
Compare
yogurtandjam
changed the title
| chainId != null && | ||
| dydxTokenDemon != null && | ||
| squidIntegratorId != null | ||
| ) { |
There was a problem hiding this comment.
Nit: Can we add an else to call Logger.e { } to log an error? Same with other functions.
There was a problem hiding this comment.
hmm wait do you mean if we don't have all the args? are we sure that's what we want?
any transfer state change will fire off a retrieve deposit/withdrawal route function even if it doesn't make sense to do so, like when we clear the modal after submitting a withdraw/deposit. If we add an error log we'll be logging an error for all of those instances
There was a problem hiding this comment.
perhaps we should just error() out for things like fromChain and nativeChainUSDCDenom being null.
| } | ||
| val chainId = helper.environment.dydxChainId | ||
| val squidIntegratorId = helper.environment.squidIntegratorId | ||
| val dydxTokenDemon = helper.environment.tokens["usdc"]?.denom |
There was a problem hiding this comment.
Can we use nativeTokenDemon instead? Legal prefers "native" over "dydx" in our codebase.
Also, tokens["usdc"] would be the UDSC instead Native token, so the variable name should change accordingly.
There was a problem hiding this comment.
ah got it thanks! this was c/p'ed from another method and was confused by this naming as well. thanks :)
There was a problem hiding this comment.
renamed to nativeChainUSDCDenom since nativeTokenDenom isn't quite accurate.
| "Content-Type" to "application/json", | ||
| ) | ||
| helper.post(url, header, body.toJsonPrettyPrint()) { _, response, code, headers -> | ||
| if (response != null) { |
There was a problem hiding this comment.
Might want to check the http code as well.
There was a problem hiding this comment.
we actually accept 4XX codes and save the error msg into state so the users can see why their withdraw/deposit attempts are failing!
| @@ -39,6 +39,8 @@ internal fun TradingStateMachine.squidV2SdkInfo(payload: String): StateChanges? | |||
| } | |||
| } | |||
|
|
|||
| @Suppress("ForbiddenComment") | |||
There was a problem hiding this comment.
nit you can make a ticket and use the ticket number instead of TODO
| val header = iMapOf( | ||
| "Content-Type" to "application/json", | ||
| ) | ||
| helper.post(url, header, body.toJsonPrettyPrint()) { _, response, code, headers -> |
There was a problem hiding this comment.
why .toJsonPrettyPrint() ?
There was a problem hiding this comment.
post body arg accepts a string - just following the convention we've been using thus far in other fns for str formatting
src/commonMain/kotlin/exchange.dydx.abacus/state/v2/supervisor/OnboardingSupervisor.kt
Outdated
Show resolved
Hide resolved
src/commonMain/kotlin/exchange.dydx.abacus/state/v2/supervisor/OnboardingSupervisor.kt
Outdated
Show resolved
Hide resolved
followup to: #415
adds top level withdrawal methods.
retrieveSkipWithdrawalRouteNonCCTP matches retrieveWithdrawalRouteV1
retrieveSkipWithdrawalRouteCCTP matches retrieveWithdrawalRouteV2
retrieveSkipWithdrawalRouteExchange matches retrieveWithdrawalRouteNoble
cctpToNobleSkip matches cctpToNobleSquid (renamed from cctpToNoble)
This PR is followed up by https://github.com/dydxprotocol/v4-abacus/pull/443/files