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

Pow check of events incoming #341

Merged
merged 2 commits into from
Aug 8, 2024
Merged

Pow check of events incoming #341

merged 2 commits into from
Aug 8, 2024

Conversation

arkanoider
Copy link
Collaborator

@arkanoider arkanoider commented Aug 7, 2024

Hi @grunch ,

mostrod now checks the POW of incoming events.
Settings file now in [mostro] section has also pow value ( 0 means no pow ).

Added also pow requested by mostro in info event.

@arkanoider arkanoider mentioned this pull request Aug 7, 2024
src/nip33.rs Outdated Show resolved Hide resolved
Copy link
Member

@grunch grunch left a comment

Choose a reason for hiding this comment

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

I tested it with pow = 1, then I publish a new order with mostro-cli last commit and mostrod publish the order

If I try to cancel the event now I get INFO mostrod::app: Not POW verified event! and it doens't cancel, that is ok

@arkanoider
Copy link
Collaborator Author

arkanoider commented Aug 8, 2024

I tested it with pow = 1, then I publish a new order with mostro-cli last commit and mostrod publish the order

If I try to cancel the event now I get INFO mostrod::app: Not POW verified event! and it doens't cancel, that is ok

I noticed this too...for example i tried with pow=10 on mostrod and pow=8 on cli and it can pass because could happen that you create an event on cli with id like:

0x001efrys....

0 - 4 bits to 0 so 4
0 - 4 bits to 0 so 4 other 0... 8 totally...
1 - in binary is 0010 so you have 10 zero leading bits and it passes...

I think that we need to think to pow as some robust one like 30 so cannot have a random lucky shot...if you put pow=1 on mostrod it's not so incredible that an event can be generated with the first digit that is zero.

This is my opinion, I can ask confirmation to @yukibtc in case or just look at the code.

@grunch
Copy link
Member

grunch commented Aug 8, 2024

I tested it with pow = 1, then I publish a new order with mostro-cli last commit and mostrod publish the order
If I try to cancel the event now I get INFO mostrod::app: Not POW verified event! and it doens't cancel, that is ok

I noticed this too...for example i tried with pow=10 on mostrod and pow=8 on cli and it can pass because could happen that you create an event on cli with id like:

0x001efrys....

0 - 4 bits to 0 so 4 0 - 4 bits to 0 so 4 other 0... 8 totally... 1 - in binary is 0010 so you have 10 zero leading bits and it passes...

I think that we need to think to pow as some robust one like 30 so cannot have a random lucky shot...if you put pow=1 on mostrod it's not so incredible that an event can be generated with the first digit that is zero.

This is my opinion, I can ask confirmation to @yukibtc in case or just look at the code.

I see, great job, let's merge this 👶🏽

Copy link
Member

@grunch grunch left a comment

Choose a reason for hiding this comment

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

🐤

@grunch grunch merged commit 12f14bb into main Aug 8, 2024
1 check passed
@arkanoider arkanoider deleted the pow branch August 8, 2024 18:19
@yukibtc
Copy link
Contributor

yukibtc commented Aug 8, 2024

I tested it with pow = 1, then I publish a new order with mostro-cli last commit and mostrod publish the order
If I try to cancel the event now I get INFO mostrod::app: Not POW verified event! and it doens't cancel, that is ok

I noticed this too...for example i tried with pow=10 on mostrod and pow=8 on cli and it can pass because could happen that you create an event on cli with id like:

0x001efrys....

0 - 4 bits to 0 so 4 0 - 4 bits to 0 so 4 other 0... 8 totally... 1 - in binary is 0010 so you have 10 zero leading bits and it passes...

I think that we need to think to pow as some robust one like 30 so cannot have a random lucky shot...if you put pow=1 on mostrod it's not so incredible that an event can be generated with the first digit that is zero.

This is my opinion, I can ask confirmation to @yukibtc in case or just look at the code.

Correct. A POW of 24-32 could be good.


Exists an option in the Client to set a min POW. Could be more efficient to use that instead of Event::check_pow, so the event will be almost immediately discarded instead of being fully parsed, verified and so on. The SDK execute these steps when receive a new relay message:

  • Parse raw relay message
  • Check message/event limits
  • Partially parse event (only ID and pubkey)
  • Check blacklist (in-memory)
  • Check POW <-- HERE
  • Parse missing event fields and compose full event
  • Check if it's expired
  • Check if it's valid
  • Check if exists in db or if was already seen
  • Send notification to channels

You can set POW in Client with:

let client = Client::default();
client.update_min_pow_difficulty(28);

// OR

let opts = Options::default().min_pow(28);
let client = Client::with_opts(signer, opts);

Will be logged an error message when receive an event with low difficulty.

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