Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

Payments authentication toggle setting #340

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

Jeiwan
Copy link

@Jeiwan Jeiwan commented May 11, 2020

This PR resolves #269

Description

This PR adds a new setting: Authenticate payments. It's on by default and, when switched off, payments don't require authentication (no need to type PIN or scan biometrics). For security reasons, switching it off requires authentication.

Motivation and Context

Sending many payments can be annoying because each of them requires a PIN or a biometrics scan. Users can opt out of authenticating every payment.

How Has This Been Tested?

Tested in a simulator connected to Simnet. Sent multiple payments with 'Authenticate payments' setting on and off and with fake biometrics authentication on and off .

Screenshots (if appropriate):


Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@Jeiwan Jeiwan changed the title Payments PIN protection setting Payments authentication toggle setting May 12, 2020
@codecov-io
Copy link

Codecov Report

Merging #340 into master will decrease coverage by 3.73%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #340      +/-   ##
=========================================
- Coverage   10.27%   6.54%   -3.74%     
=========================================
  Files         245     234      -11     
  Lines        8629    8330     -299     
=========================================
- Hits          887     545     -342     
- Misses       7742    7785      +43     
Impacted Files Coverage Δ
Library/Generated/strings.swift 6.97% <ø> (ø)
...y/Scenes/ModalDetail/Send/SendViewController.swift 0.00% <0.00%> (ø)
...y/Scenes/Settings/GroupedTableViewController.swift 0.00% <0.00%> (ø)
...ngs/Items/PaymentsAuthenticationSettingsItem.swift 0.00% <0.00%> (ø)
Library/Scenes/Settings/Settings.swift 0.00% <0.00%> (ø)
...brary/Scenes/Settings/SettingsViewController.swift 0.00% <0.00%> (ø)
...htning/Services/Misc/BlockChainHeightUpdater.swift 80.43% <0.00%> (-15.22%) ⬇️
Lightning/Tests/DateEstimatorTests.swift
Library/Tests/SyncPercentageEstimatorTests.swift
Lightning/Tests/LightningInvoiceURITests.swift
... and 10 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0c914b9...46b7a2d. Read the comment docs.

@Jeiwan Jeiwan marked this pull request as ready for review May 13, 2020 13:06
@Jeiwan
Copy link
Author

Jeiwan commented May 13, 2020

That coverage report is wrong, I didn't change anything in BlockChainHeightUpdater.swift.

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

Successfully merging this pull request may close these issues.

Pin opt-out
2 participants