-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Create payment method "Faster Payments System (SBP)" for Russian Ruble #7255
Merged
HenrikJannsen
merged 3 commits into
bisq-network:master
from
cparke2:create-sbp-payment-method
Sep 26, 2024
Merged
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
a136f79
Add new payment method "Faster Payments System (SBP)" for Russian Ruble
cparke2 a0f24fe
Add new payment method "Faster Payments System (SBP)" for Russian Ruble
cparke2 0bc6e63
Add new payment method "Faster Payments System (SBP)" for Russian Ruble
cparke2 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,78 @@ | ||
| /* | ||
| * This file is part of Bisq. | ||
| * | ||
| * Bisq is free software: you can redistribute it and/or modify it | ||
| * under the terms of the GNU Affero General Public License as published by | ||
| * the Free Software Foundation, either version 3 of the License, or (at | ||
| * your option) any later version. | ||
| * | ||
| * Bisq is distributed in the hope that it will be useful, but WITHOUT | ||
| * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or | ||
| * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero General Public | ||
| * License for more details. | ||
| * | ||
| * You should have received a copy of the GNU Affero General Public License | ||
| * along with Bisq. If not, see <http://www.gnu.org/licenses/>. | ||
| */ | ||
|
|
||
| package bisq.core.payment; | ||
|
|
||
| import bisq.core.locale.FiatCurrency; | ||
| import bisq.core.locale.TradeCurrency; | ||
| import bisq.core.payment.payload.SbpAccountPayload; | ||
| import bisq.core.payment.payload.PaymentAccountPayload; | ||
| import bisq.core.payment.payload.PaymentMethod; | ||
|
|
||
| import java.util.List; | ||
|
|
||
| import lombok.EqualsAndHashCode; | ||
| import lombok.NonNull; | ||
|
|
||
| @EqualsAndHashCode(callSuper = true) | ||
| public final class SbpAccount extends PaymentAccount { | ||
|
|
||
| public static final List<TradeCurrency> SUPPORTED_CURRENCIES = List.of(new FiatCurrency("RUB")); | ||
|
|
||
| public SbpAccount() { | ||
| super(PaymentMethod.SBP); | ||
| setSingleTradeCurrency(SUPPORTED_CURRENCIES.get(0)); | ||
| } | ||
|
|
||
| @Override | ||
| protected PaymentAccountPayload createPayload() { | ||
| return new SbpAccountPayload(paymentMethod.getId(), id); | ||
| } | ||
|
|
||
| @Override | ||
| public @NonNull List<TradeCurrency> getSupportedCurrencies() { | ||
| return SUPPORTED_CURRENCIES; | ||
| } | ||
|
|
||
| public String getMessageForAccountCreation() { | ||
| return "payment.sbp.info.account"; | ||
| } | ||
|
|
||
| public void setMobileNumber(String mobileNumber) { | ||
| ((SbpAccountPayload) paymentAccountPayload).setMobileNumber(mobileNumber); | ||
| } | ||
|
|
||
| public String getMobileNumber() { | ||
| return ((SbpAccountPayload) paymentAccountPayload).getMobileNumber(); | ||
| } | ||
|
|
||
| public void setBankName(String bankName) { | ||
| ((SbpAccountPayload) paymentAccountPayload).setBankName(bankName); | ||
| } | ||
|
|
||
| public String getBankName() { | ||
| return ((SbpAccountPayload) paymentAccountPayload).getBankName(); | ||
| } | ||
|
|
||
| public void setHolderName(String holderName) { | ||
| ((SbpAccountPayload) paymentAccountPayload).setHolderName(holderName); | ||
| } | ||
|
|
||
| public String getHolderName() { | ||
| return ((SbpAccountPayload) paymentAccountPayload).getHolderName(); | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
129 changes: 129 additions & 0 deletions
129
core/src/main/java/bisq/core/payment/payload/SbpAccountPayload.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,129 @@ | ||
| /* | ||
| * This file is part of Bisq. | ||
| * | ||
| * Bisq is free software: you can redistribute it and/or modify it | ||
| * under the terms of the GNU Affero General Public License as published by | ||
| * the Free Software Foundation, either version 3 of the License, or (at | ||
| * your option) any later version. | ||
| * | ||
| * Bisq is distributed in the hope that it will be useful, but WITHOUT | ||
| * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or | ||
| * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero General Public | ||
| * License for more details. | ||
| * | ||
| * You should have received a copy of the GNU Affero General Public License | ||
| * along with Bisq. If not, see <http://www.gnu.org/licenses/>. | ||
| */ | ||
|
|
||
| package bisq.core.payment.payload; | ||
|
|
||
| import bisq.core.locale.Res; | ||
|
|
||
| import com.google.protobuf.Message; | ||
|
|
||
| import org.apache.commons.lang3.ArrayUtils; | ||
|
|
||
| import java.nio.charset.StandardCharsets; | ||
|
|
||
| import java.util.HashMap; | ||
| import java.util.Map; | ||
|
|
||
| import lombok.EqualsAndHashCode; | ||
| import lombok.Getter; | ||
| import lombok.Setter; | ||
| import lombok.ToString; | ||
| import lombok.extern.slf4j.Slf4j; | ||
|
|
||
| @EqualsAndHashCode(callSuper = true) | ||
| @ToString | ||
| @Setter | ||
| @Getter | ||
| @Slf4j | ||
| public final class SbpAccountPayload extends PaymentAccountPayload implements PayloadWithHolderName { | ||
| private String mobileNumber = ""; | ||
| private String holderName = ""; | ||
| private String bankName = ""; | ||
|
|
||
| public SbpAccountPayload(String paymentMethod, String id) { | ||
| super(paymentMethod, id); | ||
| } | ||
|
|
||
|
|
||
| /////////////////////////////////////////////////////////////////////////////////////////// | ||
| // PROTO BUFFER | ||
| /////////////////////////////////////////////////////////////////////////////////////////// | ||
|
|
||
| private SbpAccountPayload(String paymentMethod, | ||
| String id, | ||
| String mobileNumber, | ||
| String holderName, | ||
| String bankName, | ||
| long maxTradePeriod, | ||
| Map<String, String> excludeFromJsonDataMap) { | ||
| super(paymentMethod, | ||
| id, | ||
| maxTradePeriod, | ||
| excludeFromJsonDataMap); | ||
|
|
||
| this.mobileNumber = mobileNumber; | ||
| this.holderName = holderName; | ||
| this.bankName = bankName; | ||
| } | ||
|
|
||
| @Override | ||
| public Message toProtoMessage() { | ||
| return getPaymentAccountPayloadBuilder() | ||
| .setSbpAccountPayload(protobuf.SbpAccountPayload.newBuilder() | ||
| .setMobileNumber(mobileNumber) | ||
| .setHolderName(holderName) | ||
| .setBankName(bankName)) | ||
| .build(); | ||
| } | ||
|
|
||
| public static SbpAccountPayload fromProto(protobuf.PaymentAccountPayload proto) { | ||
| return new SbpAccountPayload(proto.getPaymentMethodId(), | ||
| proto.getId(), | ||
| proto.getSbpAccountPayload().getMobileNumber(), | ||
| proto.getSbpAccountPayload().getHolderName(), | ||
| proto.getSbpAccountPayload().getBankName(), | ||
| proto.getMaxTradePeriod(), | ||
| new HashMap<>(proto.getExcludeFromJsonDataMap())); | ||
| } | ||
|
|
||
|
|
||
| /////////////////////////////////////////////////////////////////////////////////////////// | ||
| // API | ||
| /////////////////////////////////////////////////////////////////////////////////////////// | ||
|
|
||
| @Override | ||
| public String getPaymentDetails() { | ||
| return Res.get(paymentMethodId) + " - " + | ||
| Res.getWithCol("payment.account.owner.sbp") + " " + holderName + ", " + | ||
| Res.getWithCol("payment.mobile") + " " + mobileNumber + ", " + | ||
| Res.getWithCol("payment.bank.name") + " " + bankName; | ||
| } | ||
|
|
||
| @Override | ||
| public String getPaymentDetailsForTradePopup() { | ||
| return Res.getWithCol("payment.account.owner.sbp") + " " + holderName + "\n" + | ||
| Res.getWithCol("payment.mobile") + " " + mobileNumber + "\n" + | ||
| Res.getWithCol("payment.bank.name") + " " + bankName; | ||
| } | ||
|
|
||
| @Override | ||
| public byte[] getAgeWitnessInputData() { | ||
| // We don't add holderName because we don't want to break age validation if the user recreates an account with | ||
| // slight changes in holder name (e.g. add or remove middle name) | ||
| return super.getAgeWitnessInputData( | ||
| ArrayUtils.addAll( | ||
| mobileNumber.getBytes(StandardCharsets.UTF_8), | ||
| bankName.getBytes(StandardCharsets.UTF_8) | ||
| ) | ||
| ); | ||
| } | ||
|
|
||
| @Override | ||
| public String getOwnerId() { | ||
| return holderName; | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why use all cap for Russia?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will merge it as it is, if you agree to use "Russia" instead just add it in a new PR...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One big concern in the discussion thread was that the name of this payment method (in English) is almost identical to the existing "Faster Payments" payment method. Added "(SBP)" to help it stand out in the list for people looking for it as SBP. Still, to emphasize to anyone clicking it intending for the other "Faster Payments" method, I used all caps so they'd understand quickly this is not what they're looking for.
I certainly can remove the emphasis of RUSSIA if it seems like overkill. Or, add some other text in the pop-up to clearly and unambiguously differentiate the two payment methods (perhaps on both payment methods?). Really wanted to get some input or response from @pazza83 in the discussion thread on this, but he's mostly avoided my pings to give a comment so I was left on my own.
I am working on adding Russian translations, so certainly could change this however the team feels appropriate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah ok. Yes I agree that both methods using a similar name might be confusing. I would though not do it by all caps for Russia, but rather to change the name of the payment method to maybe: SBP-Faster Payments System or Faster Payments System Russia (SBP)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another thought is to change the display name of the existing "Faster Payments" to "Faster Payments (UK)". I don't think that is a compatibility issue, as the Bisq1 accounts identify their associated payment method with an enumeration value, and once created, the display name is always a "custom name" so it won't change for anyone who already has it.
As for the new payment method from Russia, I was trying to avoid too many acronyms in the name, but "SBP" is definitely necessary in the English version or people might not realize that is what it is. English transliteration of the name in Russian is very cheesy too. Maybe to do that and still be consistent: "Faster Payments System-SBP (Russia)"?
There also is a strange set of payment methods prefixed with "India", so that is another approach that also could be used to differentiate the two systems.
I have no problem removing the all caps, that's fine. The existing Faster Payments has no informational pop-up message at all, and further as both payment methods are locked to their respective currencies, so someone choosing the wrong one isn't going to get far...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes the display string should not have any impact on anything. So Faster Payments (UK) is ok as well.
"Faster Payments System-SBP (Russia)" - sounds ok to me.
Yes I think that helps ppl to find quickly their relevant method.
Yes we use popups only if needed. Too many anyway....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, so it's actually going to be (in English):
"Faster Payment System (UK)"
https://www.wearepay.uk/what-we-do/payment-systems/faster-payment-system/
"Faster Payments System-SBP (Russia)"
https://www.cbr.ru/eng/psystem/sfp/
I'll keep the SBP pop-up but remove the all caps RUSSIA since the naming convention already makes that pretty clear.
Separately, I am also adding some Russian translations currently missing.
The new pull requests should be ready in about a week. Any idea when the next release is scheduled?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know but I guess it will take a bit longer for the release. In doubt you can make smaller PRs so that we get in at least parts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Faster Payments System-SBP (Russia) works for me also
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wiki has been added https://bisq.wiki/Faster_Payments_System_SBP