refactor: Return `util::Result` from WalletLoader methods #25616

w0xlt · 2022-07-14T22:54:47Z

This PR adds a method that implement common logic to WalletLoader methods and change them to return BResult<std::unique_ptr<Wallet>>.

Motivation: #25594 changed restoreWallet to return BResult but this method shares a common pattern with createWallet and loadWallet. This PR keeps the same pattern to all WalletLoader methods.

furszy

left a quick review

furszy · 2022-07-14T23:24:27Z

src/qt/walletcontroller.cpp

+        auto wallet{node().walletLoader().loadWallet(path, m_warning_message)};

-        if (wallet) m_wallet_model = m_wallet_controller->getOrCreateWallet(std::move(wallet));
+        m_error_message = wallet ? bilingual_str{} : wallet.GetError();
+
+        if (wallet) m_wallet_model = m_wallet_controller->getOrCreateWallet(wallet.ReleaseObj());


Suggested change

auto wallet{node().walletLoader().loadWallet(path, m_warning_message)};

if (wallet) m_wallet_model = m_wallet_controller->getOrCreateWallet(std::move(wallet));

m_error_message = wallet ? bilingual_str{} : wallet.GetError();

if (wallet) m_wallet_model = m_wallet_controller->getOrCreateWallet(wallet.ReleaseObj());

auto wallet{node().walletLoader().loadWallet(path, m_warning_message)};

if (wallet) m_wallet_model = m_wallet_controller->getOrCreateWallet(wallet.ReleaseObj());

else m_error_message = wallet.GetError();

Done in 948651d.

furszy · 2022-07-14T23:33:26Z

src/qt/walletcontroller.cpp

+        auto wallet{node().walletLoader().createWallet(name, m_passphrase, flags, m_warning_message)};

-        if (wallet) m_wallet_model = m_wallet_controller->getOrCreateWallet(std::move(wallet));
+        m_error_message = wallet ? bilingual_str{} : wallet.GetError();
+
+        if (wallet) m_wallet_model = m_wallet_controller->getOrCreateWallet(wallet.ReleaseObj());


Suggested change

auto wallet{node().walletLoader().createWallet(name, m_passphrase, flags, m_warning_message)};

if (wallet) m_wallet_model = m_wallet_controller->getOrCreateWallet(std::move(wallet));

m_error_message = wallet ? bilingual_str{} : wallet.GetError();

if (wallet) m_wallet_model = m_wallet_controller->getOrCreateWallet(wallet.ReleaseObj());

auto res_wallet{node().walletLoader().createWallet(name, m_passphrase, flags, m_warning_message)};

if (res_wallet) m_wallet_model = m_wallet_controller->getOrCreateWallet(res_wallet.ReleaseObj());

else m_error_message = res_wallet.GetError();

Done in 3c96b25.

w0xlt · 2022-07-15T02:34:08Z

Thanks @furszy for the review. The suggestions were addressed in 948651d and 3c96b25.

maflcko · 2022-07-15T08:00:50Z

src/qt/walletcontroller.cpp


-        if (wallet) m_wallet_model = m_wallet_controller->getOrCreateWallet(std::move(wallet));
+        if (wallet) m_wallet_model = m_wallet_controller->getOrCreateWallet(wallet.ReleaseObj());
+        else m_error_message = wallet.GetError();


Below I used m_error_message = wallet ? bilingual_str{} : wallet.GetError();

It would be good to use the same code consistently everywhere.

Done in 7cbfa13. Thanks for the review.

I think the code change was not correctly pushed.
The commit 7cbfa13 still displays the line:

else m_error_message = wallet.GetError();

and not:

m_error_message = wallet ? bilingual_str{} : wallet.GetError();

@shaavan I said the suggestion was addressed because I understand it's about keeping all code consistent across all commits.

I had originally defined all m_error_message same way as done in #25594: m_error_message = wallet ? bilingual_str{} : wallet.GetError();.

So #25616 (comment) and #25616 (comment) suggested changing them to else m_error_message = res_wallet.GetError(); but I had forgotten to change it to restoreWallet(). 7cbfa13 fixed this.

If I understand the BResult interface correctly, it seems to be the clearest option, as BResult<T>::GetError() will return the default bilingual_str{} : value if T is not set.

However, both approaches work. If the reviewers have a strong opinion about one of them, I can change it.

BResult is essentially an std::variant wrapper. It stores one object OR the other (succeed or failure). Not both.

So, it is redundant to ask m_error_message = wallet ? bilingual_str{} : wallet.GetError(); first and then ask if succeeded in another if block.

m_error_message = wallet ? bilingual_str{} : wallet.GetError(); was intentional to reset the member that stores the error, instead of leaving the previous value, if no error occurred. However, I am fine changing it to something else.

Yeah, I double checked it before comment. The error message is only set here after calling the backend method. So, the string will always be empty if no error occurs.

Is it possible to load a wallet with an error and then load a wallet without error on the same walletcontroller?

Oh, I missed the comment, sorry Marko.

Is it possible to load a wallet with an error and then load a wallet without error on the same walletcontroller?

You mean, restoring or opening a wallet?

Because if that is the case, there shouldn't be any problem. Every time that we restore or open a wallet we create a new WalletControllerActivity subclass which encapsulates the error and warning messages (it's a single shot class. It gets deleted as soon as it finishes processing the action).

DrahtBot · 2022-07-15T10:28:52Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#25722 (refactor: Use util::Result class for wallet loading by ryanofsky)
#25656 (refactor: wallet: return util::Result from GetReservedDestination methods by theStack)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

w0xlt · 2022-07-15T16:56:38Z

#25616 (comment) addressed in 7cbfa13.

aureleoules

ACK 7cbfa13.
I think this is a good use-case of BResult.

nit: Shouldn't the commits be merged into one as the changes are small and related?

aureleoules · 2022-07-18T10:29:43Z

src/wallet/interfaces.cpp

@@ -549,8 +549,15 @@ class WalletLoaderImpl : public WalletLoader
    void stop() override { return StopWallets(m_context); }
    void setMockTime(int64_t time) override { return SetMockTime(time); }

+    //! Return a wallet interface object or an error wrapped in a BResult instance
+    BResult<std::unique_ptr<Wallet>> makeWallet(const std::shared_ptr<CWallet>& _wallet, bilingual_str& error)


can makeWallet be renamed to makeWalletResult or something else? Having both createWallet and makeWallet is a bit confusing.

Done in 087959d. Thanks.

shaavan

Concept ACK

I agree with the idea of using BResult as the return type for walletloader method, as it ensures that the return value has either the result value (if succeed) or the error value (if failure) and never both.
The code changes look clean and concise. I want to look further into this comment before ACKing the correctness of code change.

Is it possible to load a wallet with an error and then load a wallet without an error on the same walletcontroller?

In the meantime, I would suggest squashing the commits together.

w0xlt · 2022-07-22T22:09:51Z

#25616 (review) and #25616 (review) were addressed in 087959d.

shaavan

ACK 087959d

Changes since my last review:

Renamed makeWallet -> makeWalletResult.
Squashed commits.

As per @furszy explanation in this comment, since the WalletControllerActivity class gets deleted after the loading of a wallet finishes, it is not possible to have an error message stored from a previous erroneous loading of a wallet.

Hence it is not necessary to manually reset bilingual_str{}.

Thanks, @furszy, for the explanation.

jonatack

Approach ACK

jonatack · 2022-08-05T08:46:44Z

src/qt/walletcontroller.cpp


-        if (wallet) m_wallet_model = m_wallet_controller->getOrCreateWallet(std::move(wallet));
+        if (res_wallet) m_wallet_model = m_wallet_controller->getOrCreateWallet(res_wallet.ReleaseObj());
+        else m_error_message = res_wallet.GetError();


- if (res_wallet) m_wallet_model = m_wallet_controller->getOrCreateWallet(res_wallet.ReleaseObj()); - else m_error_message = res_wallet.GetError(); + if (res_wallet) { + m_wallet_model = m_wallet_controller->getOrCreateWallet(res_wallet.ReleaseObj()); + } else { + m_error_message = res_wallet.GetError(); + }

Any reason why you've changed the variable name from wallet to res_wallet here? It's wallet in the identical code sections below.

jonatack · 2022-08-05T08:47:32Z

src/qt/walletcontroller.cpp


-        if (wallet) m_wallet_model = m_wallet_controller->getOrCreateWallet(std::move(wallet));
+        if (wallet) m_wallet_model = m_wallet_controller->getOrCreateWallet(wallet.ReleaseObj());
+        else m_error_message = wallet.GetError();


- if (wallet) m_wallet_model = m_wallet_controller->getOrCreateWallet(wallet.ReleaseObj()); - else m_error_message = wallet.GetError(); + if (wallet) { + m_wallet_model = m_wallet_controller->getOrCreateWallet(wallet.ReleaseObj()); + } else { + m_error_message = wallet.GetError(); + }

jonatack · 2022-08-05T08:48:43Z

src/qt/walletcontroller.cpp

        if (wallet) m_wallet_model = m_wallet_controller->getOrCreateWallet(wallet.ReleaseObj());
+        else m_error_message = wallet.GetError();


- if (wallet) m_wallet_model = m_wallet_controller->getOrCreateWallet(wallet.ReleaseObj()); - else m_error_message = wallet.GetError(); + if (wallet) { + m_wallet_model = m_wallet_controller->getOrCreateWallet(wallet.ReleaseObj()); + } else { + m_error_message = wallet.GetError(); + }

w0xlt · 2022-08-07T05:21:53Z

Thanks for the review @shaavan, but a rebase was needed.

Rebased and changed the code to use util::Result which replaced the BResult.

@jonatack I used the suggested format in this new commit.

jonatack

Review/debug build/unit tests ACK f179998

jonatack · 2022-08-07T14:39:23Z

src/interfaces/wallet.h

@@ -88,7 +88,7 @@ class Wallet
    virtual std::string getWalletName() = 0;

    // Get a new address.
-    virtual util::Result<CTxDestination> getNewDestination(const OutputType type, const std::string label) = 0;
+    virtual util::Result<CTxDestination> getNewDestination(const OutputType type, const std::string& label) = 0;


Thanks for picking up #25721 (comment) here and fixing up indentation in this file.

The wallet method still creates a copy?

$ git grep GetNewDestination -- '*wallet.h' src/wallet/wallet.h: util::Result<CTxDestination> GetNewDestination(const OutputType type, const std::string label);

jonatack · 2022-08-07T14:43:52Z

src/wallet/interfaces.cpp

    {
        DatabaseOptions options;
        DatabaseStatus status;
        ReadDatabaseArgs(*m_context.args, options);
        options.require_existing = true;
-        return MakeWallet(m_context, LoadWallet(m_context, name, true /* load_on_start */, options, status, error, warnings));
+        bilingual_str error;
+        util::Result<std::unique_ptr<Wallet>> wallet{MakeWallet(m_context, LoadWallet(m_context, name, true /* load_on_start */, options, status, error, warnings))};


nit if you repush, here and in createWallet() above, while touching this line can use Clang-tidy named arg format (/*load_on_start=*/true) like in restoreWallet() below.

jonatack · 2022-08-07T14:47:26Z

src/wallet/interfaces.cpp

+        if (!wallet) {
+            return util::Error{error};
+        }
+        return wallet;


nit, could save 9 lines by replacing the above 4 lines in each of the 3 methods with identical logic in one line: return wallet ? wallet : util::Error{error}; (feel free to ignore)

That would be good, but applying this change will result in the error below.
I'm not sure about the reason for the error.

wallet/interfaces.cpp:577:25: error: call to implicitly-deleted copy constructor of 'util::Result<std::unique_ptr<Wallet>>' return wallet ? wallet : util::Error{error}; ^~~~~~ ./util/result.h:38:36: note: copy constructor of 'Result<std::unique_ptr<interfaces::Wallet>>' is implicitly deleted because field 'm_variant' has a deleted copy constructor std::variant<bilingual_str, T> m_variant;

Ah, std::unique_ptr is move constructible and move assignable but not copy constructible or copy assignable. So this would work: return wallet ? std::move(wallet) : util::Error{error};

Thanks. Done in 466374f.

So return wallet moves the object and return wallet ? wallet : util::Error{error}; copies the object. Interesting.

It might be that return with a ternary doesn't benefit from RVO (return value optimization) by the compiler in the same way as with an if statement.

https://en.cppreference.com/w/cpp/language/copy_elision

Edit: benefitting from RVO could (possibly, unsure) be an argument for using the more verbose if statement.

Interesting discussion about this:

https://stackoverflow.com/questions/48250917/should-we-write-stdmove-in-the-cases-when-rvo-can-not-be-done

https://stackoverflow.com/questions/22078029/why-does-the-ternary-operator-prevent-return-value-optimization

Thanks, not a blocker but this is a topic I plan to go deeper into in order to improve my understanding.

theStack

Concept ACK

nit: Same as done in the commit message, the PR title and description should also be adapted to the latest rebase (s/BResult/util::Result/).

w0xlt · 2022-08-08T01:22:16Z

CI error seems unrelated.
interface_usdt_validation.py worked fine on my machine.

https://cirrus-ci.com/task/6722929781112832

jonatack · 2022-08-08T12:53:59Z

.vscode/settings.json

@@ -0,0 +1,81 @@
+{


Looks like this file invited itself into the last push, ACK 466374f otherwise :)

Removed. Thanks.

jonatack

ACK be13477

jonatack · 2022-08-09T06:08:51Z

src/wallet/test/wallet_tests.cpp


    CMutableTransaction mtx;
-    mtx.vout.push_back({COIN, GetScriptForDestination(dest)});
+    mtx.vout.push_back({COIN, GetScriptForDestination(*op_dest)});


Thanks for picking up #25721 (comment) here.

jonatack · 2022-08-09T06:09:42Z

src/wallet/scriptpubkeyman.cpp

@@ -1770,7 +1770,7 @@ bool DescriptorScriptPubKeyMan::GetReservedDestination(const OutputType type, bo
    } else {
        error = util::ErrorString(op_dest);
    }
-    return bool(op_dest);
+    return op_dest.has_value();


Thanks for picking up the suggestion #25721 (comment) here.

furszy

utACK be13477, just a small comment.

furszy · 2022-08-09T14:11:39Z

src/qt/walletcontroller.cpp

+        m_error_message = util::ErrorString(wallet);
+        if (wallet) {
+            m_wallet_model = m_wallet_controller->getOrCreateWallet(std::move(*wallet));
+        }


why the revert here? (and in the others as well).
the result could be an object or an error, not both.

if (wallet) m_wallet_model = something; else m_error_message = util::ErrorString(wallet);

Thinking that might be good to add an assertion inside util::ErrorString, callers should always check that the result is an error prior to convert it.

Maybe assert(std::holds_alternative<bilingual_str>(result.m_variant)); inside util::ErrorString ?

However this assertion cause a compile-time error error: static_assert failed due to requirement '__detail::__variant::__exactly_once<bilingual_str, bilingual_str, bilingual_str>' "T must occur exactly once in alternatives"

yep, that was where I was pointing to.
assert(!result.has_value()); there should work fine.

I applied the suggestion above (if (wallet) m_wallet_model = something;) in ac74799.

But regarding the assertion, I think a follow-up PR is better because it also requires changing the src/test/result_tests.cpp which expects the result of util::ErrorString be {} if the object has value This would increase the scope of this PR.

Missing brackets in the conditionals again (e.g. #25616 (comment))

shouldn't be needed:
https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md?plain=1#L77-L80

"If an if only has a single-statement then-clause, it can appear on the same line as the if, without braces. In every other case, braces are required, and the then and else clauses must appear correctly indented on a new line."

yep, aren't we having a single line statement here?

if (wallet) m_wallet_model = m_wallet_controller->getOrCreateWallet(std::move(*wallet)); else m_error_message = util::ErrorString(wallet);

furszy

ACK ac74799

w0xlt · 2022-08-10T14:38:49Z

Rebased.

jonatack · 2022-08-10T15:17:51Z

Review ACK 07df6cd

theStack

Code-review ACK 07df6cd

maflcko · 2022-08-10T17:12:11Z

src/interfaces/wallet.h

@@ -320,31 +320,31 @@ class WalletLoader : public ChainClient
 {
 public:
    //! Create new wallet.
-    virtual std::unique_ptr<Wallet> createWallet(const std::string& name, const SecureString& passphrase, uint64_t wallet_creation_flags, bilingual_str& error, std::vector<bilingual_str>& warnings) = 0;
+    virtual util::Result<std::unique_ptr<Wallet>> createWallet(const std::string& name, const SecureString& passphrase, uint64_t wallet_creation_flags, std::vector<bilingual_str>& warnings) = 0;


Would probably be best to also wrap the warnings in the result? Are you working on this @ryanofsky ?

re: #25616 (comment)

Would probably be best to also wrap the warnings in the result? Are you working on this @ryanofsky ?

It needs rebase, but yes I did this in #25722

Yeah, I meant a minimal extract without the other C++ bloat (for example Result<void>) that isn't needed for simply passing warnings

maflcko · 2022-08-10T17:12:51Z

src/qt/walletcontroller.cpp

@@ -393,8 +401,11 @@ void RestoreWalletActivity::restore(const fs::path& backup_file, const std::stri
    QTimer::singleShot(0, worker(), [this, backup_file, wallet_name] {
        auto wallet{node().walletLoader().restoreWallet(backup_file, wallet_name, m_warning_message)};

-        m_error_message = util::ErrorString(wallet);
-        if (wallet) m_wallet_model = m_wallet_controller->getOrCreateWallet(std::move(*wallet));


nit: I preferred the previous code, as it is shorter and less ambiguous

…r methods 07df6cd wallet: Return `util::Result` from WalletLoader methods (w0xlt) Pull request description: This PR adds a method that implement common logic to WalletLoader methods and change them to return `BResult<std::unique_ptr<Wallet>>`. Motivation: bitcoin#25594 changed `restoreWallet` to return `BResult` but this method shares a common pattern with `createWallet` and `loadWallet`. This PR keeps the same pattern to all WalletLoader methods. ACKs for top commit: jonatack: Review ACK 07df6cd theStack: Code-review ACK 07df6cd Tree-SHA512: 2fe321134883f7cce60206888113800afef0fa168dab184e1a8692cd21a231970eb9c25c7220ea39e5d457134002d47f0974463925db76abbf8dfcd421629c63

furszy reviewed Jul 14, 2022

View reviewed changes

DrahtBot added the Refactoring label Jul 14, 2022

w0xlt force-pushed the brrrrresult-load-create branch from 5eb9f50 to 324d3bb Compare July 15, 2022 02:30

maflcko reviewed Jul 15, 2022

View reviewed changes

DrahtBot mentioned this pull request Jul 15, 2022

util: Make BResult error a generic type instead of only bilingual_str #25601

Closed

w0xlt force-pushed the brrrrresult-load-create branch from 324d3bb to 7cbfa13 Compare July 15, 2022 15:52

aureleoules reviewed Jul 18, 2022

View reviewed changes

shaavan reviewed Jul 18, 2022

View reviewed changes

DrahtBot mentioned this pull request Jul 21, 2022

refactor: Add util::Result failure values, multiple error and warning messages #25665

Open

w0xlt force-pushed the brrrrresult-load-create branch from 7cbfa13 to 087959d Compare July 22, 2022 22:08

shaavan approved these changes Jul 23, 2022

View reviewed changes

This was referenced Jul 27, 2022

refactor: Use util::Result class for wallet loading #25722

Draft

refactor: Replace BResult with util::Result #25721

Merged

jonatack reviewed Aug 5, 2022

View reviewed changes

DrahtBot added the Needs rebase label Aug 5, 2022

w0xlt force-pushed the brrrrresult-load-create branch 2 times, most recently from 017b473 to f179998 Compare August 7, 2022 04:13

DrahtBot removed the Needs rebase label Aug 7, 2022

jonatack reviewed Aug 7, 2022

View reviewed changes

theStack reviewed Aug 7, 2022

View reviewed changes

w0xlt changed the title ~~refactor: Return BResult from WalletLoader methods~~ refactor: Return util::Result from WalletLoader methods Aug 7, 2022

w0xlt force-pushed the brrrrresult-load-create branch from f179998 to 466374f Compare August 8, 2022 00:00

w0xlt mentioned this pull request Aug 8, 2022

ci error: interface_usdt_validation.py fails #25801

Closed

DrahtBot mentioned this pull request Aug 8, 2022

refactor: wallet: return util::Result from GetReservedDestination methods #25656

Merged

jonatack reviewed Aug 8, 2022

View reviewed changes

w0xlt force-pushed the brrrrresult-load-create branch from 466374f to be13477 Compare August 8, 2022 12:58

jonatack reviewed Aug 9, 2022

View reviewed changes

furszy reviewed Aug 9, 2022

View reviewed changes

w0xlt force-pushed the brrrrresult-load-create branch from be13477 to ac74799 Compare August 9, 2022 18:17

furszy approved these changes Aug 9, 2022

View reviewed changes

w0xlt force-pushed the brrrrresult-load-create branch from ac74799 to 84b08f3 Compare August 10, 2022 02:54

DrahtBot added the Needs rebase label Aug 10, 2022

wallet: Return util::Result from WalletLoader methods

07df6cd

w0xlt force-pushed the brrrrresult-load-create branch from 84b08f3 to 07df6cd Compare August 10, 2022 14:38

DrahtBot removed the Needs rebase label Aug 10, 2022

theStack approved these changes Aug 10, 2022

View reviewed changes

maflcko merged commit f5e96ec into bitcoin:master Aug 10, 2022

maflcko reviewed Aug 10, 2022

View reviewed changes

w0xlt deleted the brrrrresult-load-create branch August 10, 2022 19:01

bitcoin locked and limited conversation to collaborators Aug 15, 2023

		if (wallet) m_wallet_model = m_wallet_controller->getOrCreateWallet(wallet.ReleaseObj());
		else m_error_message = wallet.GetError();

refactor: Return util::Result from WalletLoader methods #25616

refactor: Return util::Result from WalletLoader methods #25616

Uh oh!

Conversation

w0xlt commented Jul 14, 2022

Uh oh!

furszy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

w0xlt commented Jul 15, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

furszy Jul 16, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DrahtBot commented Jul 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Conflicts

Uh oh!

w0xlt commented Jul 15, 2022

Uh oh!

aureleoules left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shaavan left a comment

Choose a reason for hiding this comment

Uh oh!

w0xlt commented Jul 22, 2022

Uh oh!

shaavan left a comment

Choose a reason for hiding this comment

Uh oh!

jonatack left a comment

Choose a reason for hiding this comment

Uh oh!

jonatack Aug 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

w0xlt commented Aug 7, 2022

Uh oh!

jonatack left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maflcko Aug 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

refactor: Return `util::Result` from WalletLoader methods #25616

refactor: Return `util::Result` from WalletLoader methods #25616

furszy Jul 16, 2022 •

edited

Loading

DrahtBot commented Jul 15, 2022 •

edited

Loading

jonatack Aug 5, 2022 •

edited

Loading

maflcko Aug 10, 2022 •

edited

Loading

jonatack Aug 8, 2022 •

edited

Loading

jonatack Aug 8, 2022 •

edited

Loading