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.

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

Check that value_msat_total is less than or equal to the amount sent #12

Open
valcanobacon opened this issue Dec 17, 2021 · 11 comments
Open
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@valcanobacon
Copy link
Contributor

valcanobacon commented Dec 17, 2021

Nothing prevents a user from putting a value in value_msat_total which is greater than the amount actually sent. These transactions should either be ignored, flagged in the UI, or use the smaller value in the UI.

PUBKEY="03ecb3ee55ba6324d40bea174de096dc9134cb35d990235723b37ae9b5c49f4f53"
AMOUNT=33
DATA=`echo '{"action":"boost","value_msat_total":"6969696969696969","app_name":"lncli","sender_name":"ValcanoBacon","name":"Podcaster","message":"Look at this big boost!"}' |  od -A n -t x1 | sed 's/ *//g' | tr -d '\n'`
lncli sendpayment --dest=$PUBKEY --amt=$AMOUNT --keysend --data 7629169=$DATA
@daveajones
Copy link
Contributor

So you’re saying to do a test against what’s in the invoice itself and use the smaller of the two values?

@valcanobacon
Copy link
Contributor Author

Maybe that won't work either. I guess the point of value_msat_total is to record the original boost amount in case the receiver received less in case of a split. However any value can be entered. You could trust it based off of the app sent but that is spoofable. If the application included a signature but that requires getting all the apps onboard. Perhaps the UI should display both values. @kahlil

@valcanobacon
Copy link
Contributor Author

I found a spec for the proposed 7629173 spec. https://github.com/satoshisstream/satoshis.stream/blob/67f5db14fac8260341f0a4733733e3b20b75faa7/TLV_registry.md#field-7629173-proposed

Looks like it contains a signature field sig is the signature of some base64-blocks. `sig( concatenate( base64(identifier), "|", base64(sender_key), "|", base64(destinations), "|", base64(payments_json), "|" ) ) Not sure if this is actual proof or not. If it does work we could include something in the UI that indicates its verified.

I really think that this will hurt adoption if its not addressed. What If someone wants to build award or knight system on this. If there is no signature solution then boosts should only not include value_msat_total and use the actual amount received.

@caramel-chirping-decree
Copy link

caramel-chirping-decree commented Dec 31, 2021

Relying on the value_msat_total declaration in its current form is not a long term solution. Using a version of Helipad and sending hand crafted keysend payments, I was able to forge a boostagram from Breez with a note and a value that was false.

image

Since the value of value_msat_total will almost always be higher than the actual payment amount (due to routing fees and declared RSS splits) there is no real way to validate that the number is legitimate.

I understand that Adam wants real values to be read out on the show and how important numerology is to people. One potential solution is to have boostagrams sent to only one of the nodes in the split, and then the declared value can be compared to actual received value, and if it falls within a certain percentage it can pass the validity check.

For instance, I boost the Podcasting 2.0 feed, select who receives the boost from the splits. This would be a modal split, only one of the nodes could be selected per boostagram. I would select the Podcasting 2.0 node, and boost 1000 sats. Due to routing fees, only 9998 sats arrive to your node with the boostagram, but the value_msat_total would be 1000 sats. Then there can be some logic in helipad, or any other boostagram extractor, that could compare the actual total to the value_msat_total and ensure it falls into a chosen percentage range before showing it.

@daveajones
Copy link
Contributor

I can start by just showing the two values in the UI and flagging the value_msat_total in something like Red if it doesn’t fall within a sane fee percentage window?

@caramel-chirping-decree
Copy link

That may work if you can find a percentage that works for you, but it would only really work if boostagrams have no recipient split.

Otherwise I can send you 100 sats and say I sent 1000 sats and gave you only 10% of the split. Unless you can verify the transactions to the other receivers in the split, you can't verify that you were sent 10% of 1000 sats or 100% of 100 sats.

@caramel-chirping-decree
Copy link

To clarify, I don't see a problem with entrusting producers to send correct amounts and not lie, but there should be a UI setting to set to validated amounts only. I believe there will be a time where boostagrams become main stream enough to consistently get people trying to send fraudulent amounts in.

It is better to solve the issue before it becomes pervasive.

@valcanobacon
Copy link
Contributor Author

valcanobacon commented Jan 3, 2022

If you integrated the value block from the feed you could compare the percentages to see if they are reasonable.

I am starting to think we should consider value_msat_total more of a note and less of fact. I think we should display the actually amount received and the note indicating what was sent originally. Any reward system should use the actual amount received. If a standard was set and the apps communicated this well then there would be no issue. A warning based off percentages would still be helpful but can't be 100% accurate. If the apps had a mechanism to send a boost to an individual and have fees added to the amount instead of subtracted from the amount then we could meet this future use case.

@daveajones
Copy link
Contributor

Agree with this.

@SpencerPearson
Copy link

When you consider programming for #21 two things come to my mind:

  1. you're going to want to know the original boost amount to make sure when someone boosts a magic number the correct response is triggered

  2. this customized boost sound feature only further incentivizes the problem raised in the original issue here to trigger certain jingles without fully paying for them

I think the only way to do it properly is to check if the invoice matches the expected percentage split of the original boost amount . We could even check it against the sending app for more accuracy. If you see the boost is from curiocaster, you know the breakdown of what your split should be after their fee structure is applied.

If the percentage is off by some small amount, we could mark it as bogus and not respond to it in the program...keep the sats, don't play the pews. hell, we could even say only mark it bogus if the boost comes in under expected split amount but just treat original boost amount as true if it's over the expected percentage. After all there are scenarios where people can directly boost your node from certain apps, which would cause a higher than your expected percentage in the boost amount. But we have to eventually program a way to reasonably verify the original boost amount.

@valcanobacon
Copy link
Contributor Author

That check could and probably should be implemented client side with JavaScript.

@ericpp ericpp added enhancement New feature or request good first issue Good for newcomers labels May 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

5 participants