[ANCHOR-809][SEP-6] Add funding_method to SEP-6 #1582
Conversation
| "optional": false | ||
| } | ||
| } | ||
| "funding_methods": ["SEPA", "SWIFT"] |
There was a problem hiding this comment.
Does it make sense to support the type request parameter when the /info response returns funding_methods?
There was a problem hiding this comment.
The plan is to support funding_method while deprecating type. Same as discussion #1581 (comment)
| requestValidator.validateTypes( | ||
| request.getType(), sellAsset.getCode(), sellAsset.getSep6().getWithdraw().getMethods()); | ||
| fundingMethod, sellAsset.getCode(), sellAsset.getSep6().getWithdraw().getMethods()); |
There was a problem hiding this comment.
Do we not need to persist this field in the database?
There was a problem hiding this comment.
In db it's stilled saved as type. I'm not gonna make the change for now (at least in this pr) as type is a preserved word, and it's causing a lot of issue when I was trying to either drop or rename it.
| @@ -90,7 +92,8 @@ public StartDepositResponse depositExchange( | |||
| @RequestParam(value = "account") String account, | |||
| @RequestParam(value = "memo_type", required = false) String memoType, | |||
| @RequestParam(value = "memo", required = false) String memo, | |||
| @RequestParam(value = "type") String type, | |||
| @RequestParam(value = "funding_method", required = false) String fundingMethod, | |||
There was a problem hiding this comment.
In this PR's description and the protocol, funding_method is a required parameter. Is it supposed to be required?
There was a problem hiding this comment.
It 's not marked as required because we need to support both type and funding_method and they are referencing the same thing
| /** | ||
| * The fields required to initiate a deposit. | ||
| * | ||
| * <p>The only field supported by the platform is <code>type</code>. Additional fields required | ||
| * for KYC are supplied asynchronously through SEP-12 requests. | ||
| */ | ||
| Map<String, AssetInfo.Field> fields; | ||
| @Deprecated Map<String, AssetInfo.Field> fields; |
There was a problem hiding this comment.
Add comments to the SEP version that deprecates the field.
There was a problem hiding this comment.
Can you point me to where it's missing ? I believe I have marked it as deprecated
| /** | ||
| * The types of withdrawal methods supported and their fields. | ||
| * | ||
| * <p>The platform does not allow fields to be configured for withdrawal methods. Financial | ||
| * account and KYC information is supplied asynchronously through PATCH requests and SEP-12 | ||
| * requests respectively. | ||
| */ | ||
| Map<String, WithdrawType> types; | ||
| @Deprecated Map<String, WithdrawType> types; |
There was a problem hiding this comment.
Add comments to the SEP version that deprecates the field.
There was a problem hiding this comment.
Can you point me to where it's missing ? I believe I have marked it as deprecated
| String fundingMethod; | ||
|
|
||
| /** Type of deposit. Deprecated in favor of funding_method */ | ||
| @Deprecated String type; |
There was a problem hiding this comment.
Can you point me to where it's missing ? I believe I have marked it as deprecated
| String fundingMethod; | ||
|
|
||
| /** Type of deposit. Deprecated in favor of funding_method */ | ||
| @Deprecated String type; |
| String fundingMethod; | ||
|
|
||
| /** The type of withdrawal to make. Deprecated in favor of funding_method */ | ||
| @Deprecated String type; |
There was a problem hiding this comment.
Can you point me to where it's missing ? I believe I have marked it as deprecated
| String fundingMethod; | ||
|
|
||
| /** Type of withdrawal. Deprecated in favor of funding_method */ | ||
| @Deprecated String type; |
There was a problem hiding this comment.
Can you point me to where it's missing ? I believe I have marked it as deprecated
Description
/inforesponse, replacefields(in deposit) andtypes(in withdraw) withfunding_methodstypewithfunding_method, and this param will be mandatoryContext
SEP-31 changes scoped in stellar/stellar-protocol#1567
Testing
./gradlew test