Feature Request: Add support for clientDomain in BuildChallengeTx

[eduartua]

What problem does your feature solve?

The function BuildChallengeTx does not support to add the clientDomain and the clientSigningKey into the transaction built.
See here.

What would you like to see?

As you can see, the python SDK supports them:
Look at the source code, specifically the func build_challenge_transaction

It uses

:param client_domain: The domain of the client application requesting authentication
:param client_signing_key: The stellar account listed as the SIGNING_KEY on the client domain's TOML file

The after this line we have

if client_domain:
        if not client_signing_key:
            raise ValueError(
                "client_signing_key is required if client_domain is provided."
            )
        transaction_builder.append_manage_data_op(
            data_name="client_domain",
            data_value=client_domain,
            source=client_signing_key,
        )

What alternatives are there?

Is it viable to add support for them? I can send in a PR.

/txnbuild

[leighmcculloch]

👀 It appear that the Go SDK has fallen behind supporting all the capabilities that SEP-10 defines. It's missing the changes introduced in SEP-10 v3.2.0, i.e. stellar/stellar-protocol@01e0d98.

@JakeUrban Is there a rollout plan for the change to SEP-10 adding client attribution? I don't remember us discussing one in particular, or opening issues on SDKs.

[JakeUrban]

You're right, we never discussed it. Support has been added to the Python SDK though.

The changes required are:

• BuildChallengeTransaction:
  • Adding client_domain and client_signing_key optional parameters
  • Adding a ManageData operation with 'client_domain' as the key and the client_signing_key parameter value as the value if these parameters are passed
• VerifyChallengeSignatures:
  • Checking for a ManageData op with 'client_domain' as the key and checking for a signature from the operation's source account if present
  • Allowing the source of the 'client_domain' ManageData operation to not equal the server's account

I can create issues on the SDKs to implement this. Is there anything else we need to do before moving forward?

[eduartua]

Thanks @JakeUrban and @leighmcculloch Let me know the new issues to see if I can help.

[leighmcculloch]

I think this issue can serve as the issue for this repo.

The list of changes required looks good to me.