Skip to content
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

SEPA Account - editable account name and accepted countries #5930

Merged
merged 3 commits into from Jan 12, 2022
Merged

SEPA Account - editable account name and accepted countries #5930

merged 3 commits into from Jan 12, 2022

Conversation

ghost
Copy link

@ghost ghost commented Dec 20, 2021

Video:
update-account

Fixes #5692

@ghost ghost marked this pull request as ready for review December 30, 2021 10:47
@ripcurlx
Copy link
Contributor

ripcurlx commented Jan 3, 2022

@xyzmaker123 Did you try out what happens when you do this on an already signed account? I fear that this will invalidate the account signing state as it changes the hash as well.

@ghost
Copy link
Author

ghost commented Jan 4, 2022

@ripcurlx I didn't tried it on an already signed account - I don't know how to invoke this scenario on localnet dev setup. IIUC acceptedCountries are not used for hash calculation - only iban, bic, countryCode and paymentMethodId

@ghost
Copy link
Author

ghost commented Jan 4, 2022

Edge case scenario I tested:

  1. Bob has single SEPA account, and accept trades from AT and DE
  2. Alice has single SEPA account, and "country of bank set" to DE
  3. Bob created offer to buy BTC with euro
  4. Alice accepted this offer
  5. Bob removed DE from list of accepted countries
  6. Alice and Bob were able to finish transaction
    Accepted countries are checked in time of taking offer, there is no check later

@pazza83
Copy link

pazza83 commented Jan 9, 2022

Hi @xyzmaker123 if this is possible it would be great. SEPA countries have changed over the last 12 months (more have been added) so it would be good for accounts set up prior to this to be able to add more countries they are happy to trade with.

I would also propose they should also be able to edit the 'account name' as essentially this is just the nickname given to the account and only used by the users local Bisq instance.

@ghost
Copy link
Author

ghost commented Jan 9, 2022

@pazza83 This pull request support this case. These countries are unchecked when someone open edit account, and it's possible to check them.

Sure - I'll extend this pull request with possibility to edit the 'account name' (CC: @ripcurlx)

@ghost ghost changed the title Possibility to modify accepted countries SEPA Account - possibility to modify account name and accepted countries Jan 10, 2022
@ghost ghost changed the title SEPA Account - possibility to modify account name and accepted countries SEPA Account - editable account name and accepted countries Jan 10, 2022
@ghost ghost requested a review from ripcurlx January 10, 2022 09:59
@ripcurlx
Copy link
Contributor

Hi @xyzmaker123 if this is possible it would be great. SEPA countries have changed over the last 12 months (more have been added) so it would be good for accounts set up prior to this to be able to add more countries they are happy to trade with.

I would also propose they should also be able to edit the 'account name' as essentially this is just the nickname given to the account and only used by the users local Bisq instance.

Just to make sure: In this PR the SEPA countries are not extended/changed:

  • getAllSepaEuroCountries: "AT", "BE", "CY", "DE", "EE", "FI", "FR", "GR", "IE","IT", "LV", "LT", "LU", "MC", "MT", "NL", "PT", "SK", "SI", "ES", "AD", "SM", "VA"
  • getAllSepaNonEuroCountries:"BG", "HR", "CZ", "DK", "GB", "HU", "PL", "RO","SE", "IS", "NO", "LI", "CH", "JE"

Copy link
Contributor

@ripcurlx ripcurlx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK - Tested it on Regtest.

  • with signed account, where countries were changed --> account still signed
  • created offer with account, changed countries afterwards, created another offer --> both offers/trades are valid as countries are part of the offer and won't be changed at a later stage

@ripcurlx ripcurlx added this to the v1.8.1 milestone Jan 12, 2022
@bisq-github-admin-3
Copy link
Contributor

I'm still merging this PR although as Codacy has permission issues (again)

@bisq-github-admin-3 bisq-github-admin-3 merged commit 808c3ad into bisq-network:master Jan 12, 2022
@Bl5ckj5ck
Copy link

Great job @xyzmaker123 ! Thanks

@pazza83
Copy link

pazza83 commented Jan 22, 2022

Hi @xyzmaker123 thanks for this. Does this also work for SEPA Instant? Apologies just thought of this now :)

@ghost
Copy link
Author

ghost commented Jan 22, 2022

@pazza83 Yes - I also included SEPA Instant accounts during implementation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Not possible to modify accepted countries
4 participants