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

[Relay Mining] Implement Supplier driven RelayMiningDifficulty #818

Open
3 tasks
red-0ne opened this issue Sep 11, 2024 · 6 comments
Open
3 tasks

[Relay Mining] Implement Supplier driven RelayMiningDifficulty #818

red-0ne opened this issue Sep 11, 2024 · 6 comments
Assignees
Labels
miner Changes related to the Miner off-chain Off-chain business logic on-chain On-chain business logic proof Claim & Proof life cycle protocol General core protocol related changes

Comments

@red-0ne
Copy link
Contributor

red-0ne commented Sep 11, 2024

Objective

Prevent malicious Gateways from abusing on-chain RelayMiningDifficulty to get free Relays by sending low enough volumes to Suppliers that result in empty trees.

Origin Document

Potential Exploit in On-Chain Relay Mining Difficulty: Unclaimable Relays

The current on-chain RelayMiningDifficulty mechanism may be vulnerable to abuse, making Suppliers perform unclaimable work.

Malicious Gateways could exploit this by sending relays with volumes low enough to avoid triggering a relay mining process. By duplicating this process across multiple Applications, they could scale the exploit, leading to free work performed by Suppliers.

For more details, please refer to RelayMiningDifficulty-and-low-volume Notion document.

Goals

  • Implement Supplier-driven RelayMiningDifficulty
  • Disable (remove?) on-chain based RelayMiningDifficulty
  • Ensure that very low volume (a few Relays) are still claimable.

Deliverables

  • [ ][@adshmh] A single PR that:
  • [ ][@red-0ne and/or @Olshansk] Update the documentation (at least in part) to reflect these changes (TODOs, links to docs, actual updates, etc...)

Imeplementation Direction

Implement Supplier-Driven Relay Mining Difficulty

  1. Each Claim should specify the mining difficulty used to construct the SparseMerkleSumTree, which will be included in the MsgCreateClaim message.
    • This will be part of the claim object
  2. During proof verification, the difficulty value should be retrieved from the respective Claim and used to verify whether the Relay satisfies the expected difficulty.
  3. The global on-chain RelayMiningDifficulty should be updated to "suggested_difficulty"
  4. Documentation needs to be updated that on-chain difficulty is a suggestion for suppliers to reduce their memory footprint but it is ultimately a supplier decision

Additionally, this difficulty will be utilized to estimate the processing volume for the corresponding session. This is done in #771

Non-goals / Non-deliverables

  • Create new governance parameters.
  • Look for the right parameters to use.

General deliverables

  • Comments: Add/update TODOs and comments alongside the source code so it is easier to follow.
  • Testing: Add new tests (unit and/or E2E) to the test suite.
  • Documentation: Update architectural or development READMEs; use mermaid diagrams where appropriate.

Creator: @adshmh
Co-Owners: @bryanchriswhite, @red-0ne

@red-0ne red-0ne added miner Changes related to the Miner protocol General core protocol related changes off-chain Off-chain business logic on-chain On-chain business logic proof Claim & Proof life cycle labels Sep 11, 2024
@red-0ne red-0ne added this to the Shannon Beta TestNet Launch milestone Sep 11, 2024
@Olshansk
Copy link
Member

@red-0ne Will review this in detail by EOD. Please hold off from starting work on it just yet

@Olshansk
Copy link
Member

Copying this from a message on discord last week:

I just put up [1] for review as part of the ticket I'm handing off in [2] (related to proportional rewards), which is a pre-req for [3] that @Red0ne put together earlier this week.

Note that I left a lot of TODO(arash) in the PR as a guideline for where/what needs to be implemented.

This is ULTRA critical for tokenomics to work properly.

[1] #771
[2] #781
[3] #818

@Olshansk
Copy link
Member

@adshmh I wanted to call out that we need to finish #771 before this can be started.

@red-0ne

  1. I have updated the description here. PTAL and see if there are any updates/changes needed.
  2. My only open question is related to this commend. What do you mean by it?
  • Add min/max mined relays per claim parameters as constants.
  1. Do you see other risks here where the supplier claims a high difficulty but actually inserted (in part) low difficulty relays into the tree? This could potentially be an attack as well, no?

@red-0ne
Copy link
Contributor Author

red-0ne commented Sep 17, 2024

@Olshansk,

min/max mined relays per claim are described here [1]

TL;DR;

MinMinedRelaysPerClaim: Since a Supplier is the one submitting the difficulty, it could submit a claim with only the most difficult relay thus leveraging any "luck" the supplier might have, which may not correspond to the actual work done.

MaxMindedRelaysPerClaim: On the other hand prevents the Supplier from submitting a claim with the easiest difficulty leading to long proof paths.

Example
With min = 3 and max = 5
And the following served relays difficulty: 1, 2, 3, 4, 5, 6, 7, 8, 9, 20

The supplier is not allowed to to submit the following claims:

  • {difficulty: 20, minedRelays: 1}
  • {difficulty: 2, minedRelays: 9}

But is allowed to submit:

  • {difficulty: 7, minedRelays: 4}

Min and max are TBD though.

[1] RelayMiningDifficulty-and-low-volume#ProbabilisticError

@red-0ne
Copy link
Contributor Author

red-0ne commented Sep 18, 2024

Min/Max mined relays per claim are the equivalent of the targetNumRelays in the current setup

@Olshansk
Copy link
Member

@red-0ne As discussed offline, do you think this is something we can do POST Mainnet?

Olshansk added a commit that referenced this issue Sep 21, 2024
…on difficulty (#771)

## Summary

No real business logic changes, but preparation + tests + helpesr + TODOs for #781.

## Issue

- Preparation for #781
- Pre-requisite for #818

---------

Co-authored-by: Bryan White <[email protected]>
Co-authored-by: Redouane Lakrache <[email protected]>
@Olshansk Olshansk assigned red-0ne and unassigned adshmh Nov 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
miner Changes related to the Miner off-chain Off-chain business logic on-chain On-chain business logic proof Claim & Proof life cycle protocol General core protocol related changes
Projects
Status: 🔖 Ready
Development

No branches or pull requests

3 participants