Skip to content

Conversation

@knst
Copy link
Collaborator

@knst knst commented Apr 17, 2023

Issue being fixed or feature implemented

The benefits of using custom struct maybe_error is less significant since the interface TxValidationState became simpler after refactorings from bitcoin#15921 and bitcoin#17004

The refactoring of maybe_error as a class result from PR #5109 is still useful but not for case of TxValidationState.

What was done?

Unified using TxValidationState in dash's related code.

How Has This Been Tested?

Run unit/functional tests

Breaking Changes

No breaking changes, logic are same.

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have made corresponding changes to the documentation
  • I have assigned this pull request to a milestone

…lementation

Since last refactoring bitcoin#15921 and bitcoin#17004 the benefits of using maybe_error are less significants
because the interface became simpler.

In future it can be considered using universal `maybe_error` (Result) as proposed in PR dashpay#5109 instead this particular implementation
@knst knst added this to the 20 milestone Apr 17, 2023
Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

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

utACK

@UdjinM6 UdjinM6 requested a review from PastaPastaPasta April 18, 2023 20:17
@UdjinM6
Copy link

UdjinM6 commented Apr 24, 2023

@PastaPastaPasta ping

Copy link
Member

@PastaPastaPasta PastaPastaPasta left a comment

Choose a reason for hiding this comment

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

Sure, utACK

@UdjinM6 UdjinM6 merged commit e3c8dcc into dashpay:develop Apr 25, 2023
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.

3 participants