feat: adjustable pow difficulty#865
Conversation
bin/faucet/src/server/pow.rs
Outdated
| #[derive(Clone)] | ||
| pub struct PoW { | ||
| pub(crate) salt: String, | ||
| pub(crate) difficulty: Arc<Mutex<u64>>, |
There was a problem hiding this comment.
Not sure if it's worth it but we could try AtomicU64 here. Otherwise i would use tokio::sync::Mutex instead of std::sync::Mutex
bin/faucet/src/server/pow.rs
Outdated
| } | ||
|
|
||
| impl PoW { | ||
| pub fn adjust_difficulty(&mut self, sender_count: usize) { |
There was a problem hiding this comment.
we could add a doc comment on how this difficulty is adjusted to the faucet load
bin/faucet/src/server/pow.rs
Outdated
| /// The difficulty is the number of leading zeros in the hash of the seed and the solution. | ||
| const DIFFICULTY: u64 = 5; | ||
| const MAX_DIFFICULTY: u64 = 6; |
There was a problem hiding this comment.
Max difficulty of 6 leading zeros seems pretty low (i.e., it would require, on average, 64 guesses to get the right nonce). A moderately-powerful machine should be able to guess 16 leading zeros in under a second, and 20 leading zeros within a few seconds). I'd probably set the maximum at something like 24.
Also, as mentioned in another comment, ideally, PoW should be based not on the number of leading zeros but on absolute values. This way, it can be adjusted more granularly - but we can do this in a separate PR.
…ella-adjustable-pow-difficulty
igamigo
left a comment
igamigo
left a comment
There was a problem hiding this comment.
LGTM! Left some comments/questions
CHANGELOG.md
Outdated
| - Added an optional API key request parameter to skip PoW in faucet (#839). | ||
| - Block producer now emits network note information (#833). | ||
| - Introduced Network Transaction Builder (#840). | ||
| - Proof-of-Work difficulty is now adjustable based on the number of concurrent requests (#865). |
There was a problem hiding this comment.
nit (feel free to disregard):
| - Proof-of-Work difficulty is now adjustable based on the number of concurrent requests (#865). | |
| - Proof-of-Work difficulty is now adjusted based on the number of concurrent requests (#865). |
bin/faucet/src/server/get_tokens.rs
Outdated
| Query(request): Query<RawMintRequest>, | ||
| ) -> Sse<impl Stream<Item = Result<Event, Infallible>>> { | ||
| let request_guard = | ||
| RequestCounterGuard::new(server.active_requests.clone(), server.queued_requests.clone()); |
There was a problem hiding this comment.
nit: You can take &Arc<_> in RequestCounterGuard::new() and clone internally for a slightly more ergonomic API here
| /// The difficulty is adjusted based on the number of active requests. | ||
| /// The difficulty is increased by 1 for every 50 active requests. | ||
| /// The difficulty is clamped between 1 and `MAX_DIFFICULTY`. | ||
| pub fn adjust_difficulty(&self, active_requests: usize) { |
There was a problem hiding this comment.
Right now if we go back (for example) to a max queue of 200 requests the effective max difficulty is 4, right? If not, disregard this comment.
But otherwise, and maybe not for now, should this function take into account the max queue length somehow?
For example, we could map the queue length into the difficulty range by computing the filled ration of the queue and applying that to the [1..MAX_DIFFICULTY] range. This way the step interval (50 in this case) is not "coupled" with the queue length in any way and we could tweak difficulty or max queue length and expect it to work linearly (or we could decide to grow it exponentially, etc.).
There was a problem hiding this comment.
I just saw that the difficulty is adjusted based on total requests (even if the PoW does not validate), not just minting requests in progress. I was expecting it to be over the requests that validated correctly, but I think this might be fine and maybe dismisses my comment above. Not entirely sure if there is a practical difference.
In any case, would it make sense to make the step interval be configurable? Maybe as part of #878
There was a problem hiding this comment.
I replaced this logic, basically increasing the difficulty every step defined by REQUEST_QUEUE_SIZE / MAX_DIFFICULTY
bin/faucet/src/server/pow.rs
Outdated
| /// | ||
| /// The difficulty is the number of leading zeros in the hash of the seed and the solution. | ||
| const DIFFICULTY: u64 = 5; | ||
| const MAX_DIFFICULTY: u64 = 24; |
There was a problem hiding this comment.
Related to my previous comment, since the max queue length is 1000 (AFAIK, maybe something changed or I'm missing something else), I think the effective max difficulty would be 20 here.
5 participants
closes #854