Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement Manual Trigger Endpoint for Executing Splits on Specific Wallets #17

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Implement Manual Trigger Endpoint for Executing Splits on Specific Wallets #17

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b1c7549
Add execute_split post endpoint
Opago-Pay Dec 20, 2023
b90eecb
Add execute_split post endpoint
Opago-Pay Jan 11, 2024
d45a19e
Add execute_split post endpoint
Opago-Pay Dec 20, 2023
b3c2ce4
Merge remote-tracking branch 'origin/opago' into opago
Opago-Pay Jan 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 49 additions & 2 deletions tasks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,57 @@ async def on_invoice_paid(payment: Payment) -> None:
)


async def execute_split(wallet_id, amount):
Copy link
Member

Choose a reason for hiding this comment

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

uses very similar logic than on_invoice_paid should be refactored into one


targets = await get_targets(wallet_id)

if not targets:
return

total_percent = sum([target.percent for target in targets])

if total_percent > 100:
logger.error("splitpayment: total percent adds up to more than 100%")
return

logger.trace(f"splitpayments: performing split payments to {len(targets)} targets")

for target in targets:

if target.percent > 0:

amount_msat = int(amount * target.percent / 100)
memo = (
f"Split payment: {target.percent}% for {target.alias or target.wallet}"
)

if target.wallet.find("@") >= 0 or target.wallet.find("LNURL") >= 0:
safe_amount_msat = amount_msat - fee_reserve(amount_msat)
payment_request = await get_lnurl_invoice(
target.wallet, wallet_id, safe_amount_msat, memo
)
else:
_, payment_request = await create_invoice(
wallet_id=target.wallet,
amount=int(amount_msat / 1000),
internal=True,
memo=memo,
)

extra = {"tag": "splitpayments", "splitted": True}

if payment_request:
await pay_invoice(
payment_request=payment_request,
wallet_id=wallet_id,
description=memo,
extra=extra,
)


async def get_lnurl_invoice(
payoraddress, wallet_id, amount_msat, memo
payoraddress, wallet_id, amount_msat, memo
Copy link
Member

Choose a reason for hiding this comment

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

too much indentation

) -> Optional[str]:

from lnbits.core.views.api import api_lnurlscan

data = await api_lnurlscan(payoraddress)
Expand Down
6 changes: 6 additions & 0 deletions views_api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from lnbits.core.crud import get_wallet, get_wallet_for_key
from lnbits.decorators import WalletTypeInfo, check_admin, require_admin_key

from .tasks import execute_split
from . import scheduled_tasks, splitpayments_ext
from .crud import get_targets, set_targets
from .models import Target, TargetPutList
Expand All @@ -21,6 +22,11 @@ async def api_targets_get(
return targets or []


@splitpayments_ext.post("/api/v1/execute_split", status_code=HTTPStatus.OK)
async def api_execute_split(wallet_id: str, amount: int) -> None:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this should require a key (admin_key) to protect from malicious exploits

Copy link

Choose a reason for hiding this comment

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

The usecase of this is to resolve this issue: #10

For me, this is exactly what I need, and I'd prefer it to be as simple as possible. Since it only executes splits that have already been set, I cannot see any obvious exploits.

What exploit do you see this could be used for?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Anyone can trigger splits... even if the splits are for known wallets (or not... other users may have a wide range of use cases)!

Copy link

Choose a reason for hiding this comment

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

Anyone who knows the wallet can trigger a split, without having to know the admin key.

Which is the use case.

A person realizes it’s not getting paid out, so the trigger the split.

But receivers of the split shouldn’t necessarily have permission to alter the splits, which they could if they know the admin key.

I’m still not getting how it can be abused.

Copy link
Member

Choose a reason for hiding this comment

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

i agree with tal it has to require an admin key.

Copy link
Member

Choose a reason for hiding this comment

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

the issue is as wallet owner that is doing the splitting, i can chose not split 100% so the rest is staying in my wallet, now the others can basically repeat and can split the rest to themselves, if they just know my wallet_id

result = await execute_split(wallet_id, amount)
return result

@splitpayments_ext.put("/api/v1/targets", status_code=HTTPStatus.OK)
async def api_targets_set(
target_put: TargetPutList,
Expand Down