Skip to content

payment transaction data#269

Merged
Dindexx merged 8 commits intomainfrom
feature/transaction-data
Feb 27, 2025
Merged

payment transaction data#269
Dindexx merged 8 commits intomainfrom
feature/transaction-data

Conversation

@Dindexx
Copy link
Contributor

@Dindexx Dindexx commented Feb 14, 2025

Short description of what this resolves:

  • Adds support for transaction data

Changes proposed in this pull request:

Fixes: #

Signed-off-by: Kevin <kevin.dinh@lissi.id>
@Dindexx Dindexx requested a review from JoTiTu February 14, 2025 01:41
Signed-off-by: Kevin <kevin.dinh@lissi.id>
Signed-off-by: Kevin <kevin.dinh@lissi.id>
Signed-off-by: Kevin <kevin.dinh@lissi.id>
Copy link
Contributor

@JoTiTu JoTiTu left a comment

Choose a reason for hiding this comment

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

minor findings

{
public static string ToStringWithoutTrail(this System.Uri uri) => uri.ToString().TrimEnd('/');

public static Option<System.Uri> TryToParseUri(string uriString)
Copy link
Contributor

Choose a reason for hiding this comment

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

unused. Is it needed?

@@ -0,0 +1,15 @@
namespace WalletFramework.Oid4Vc.Oid4Vp.Errors;

public record AccessDeniedError : VpError
Copy link
Contributor

Choose a reason for hiding this comment

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

  • seems unused
  • const can be private

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will be used by the fw consumer

@@ -0,0 +1,15 @@
namespace WalletFramework.Oid4Vc.Oid4Vp.Errors;

public record InvalidRequestError : VpError
Copy link
Contributor

Choose a reason for hiding this comment

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

  • const can be private

/// associated with the OpenID4VP Authorization Request Url and Credentials Candidates that can be used to answer the
/// request.
/// </returns>
// Task<Validation<AuthorizationRequestCandidates>> ProcessAuthorizationRequestUri(
Copy link
Contributor

Choose a reason for hiding this comment

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

leftover code

/// Gets or Sets the Credential that is used to answer the input descriptor.
/// </summary>
public ICredential Credential { get; set; } = null!;

Copy link
Contributor

Choose a reason for hiding this comment

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

empty lines


public record InvalidTransactionDataError : VpError
{
public const string Code = "invalid_transaction_data";
Copy link
Contributor

Choose a reason for hiding this comment

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

can be private

try
{
return DeserializeObject<AuthorizationResponseCallback>(redirectUriJson);
Uri callback = DeserializeObject<AuthorizationResponseCallback>(redirectUriJson);
Copy link
Contributor

Choose a reason for hiding this comment

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

debug leftover?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

for implicit casting

try
{
return DeserializeObject<AuthorizationResponseCallback>(redirectUriJson);
Uri callbackUri = DeserializeObject<AuthorizationResponseCallback>(redirectUriJson);
Copy link
Contributor

Choose a reason for hiding this comment

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

debug leftover?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

see first message

Signed-off-by: Kevin <kevin.dinh@lissi.id>
Signed-off-by: Kevin <kevin.dinh@lissi.id>
Signed-off-by: Kevin <kevin.dinh@lissi.id>
Signed-off-by: Kevin <kevin.dinh@lissi.id>
Copy link
Contributor

@JoTiTu JoTiTu left a comment

Choose a reason for hiding this comment

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

LGTM

@Dindexx Dindexx merged commit 60531a3 into main Feb 27, 2025
3 checks passed
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.

2 participants