snarkOS icon indicating copy to clipboard operation
snarkOS copied to clipboard

Fix requests per second, proper error responses, compilation related to the curl-sys library

Open fulltimemike opened this issue 1 year ago • 3 comments

Motivation

Fixes https://github.com/AleoNet/snarkOS/issues/3402. This PR includes a patch for https://github.com/AleoNet/snarkOS/issues/3403, in order to get this code to compile.

Test Plan

Manually tested with low rps, verified that the requests per second did not become 1 after the burst quota was exhausted.

Additional Details

This is our patch fix, so feel free to adapt this code as necessary. We use per_nanosecond instead of per_second for the replenishment rate, with the goal of replenishing the burst rate in full every second. Our primary concern was the rps, which is why the 429 (Too many requests) error in particular is handled but other errors are defaulted to a 500 (Internal Server Error). Furthermore, related to issue #3403, I had to bump the rust version in order to get snarkos to compile.

fulltimemike avatar Sep 24 '24 16:09 fulltimemike

Thx! Mind removing the MSRV bump? It'll need to be updated in Cargo.toml and .circleci/config.yml as well, as well as for snarkVM. Better to make those separate PRs to be readable for everyone.

Also perhaps someone will object using the latest MSRV, so a separate PR is a better ground for discussion. Will also ask some people directly about their opinion.

vicsn avatar Sep 25 '24 14:09 vicsn

Sure, removed it. I couldn't get it to compile without updating the rust version, so you may need to update that before this is merged in

fulltimemike avatar Sep 25 '24 15:09 fulltimemike

Can you merge aleonet/staging and fix the fmt issue?

vicsn avatar Oct 23 '24 14:10 vicsn

Can you merge aleonet/staging and fix the fmt issue?

done -- feel free to change this anyway you all need to get it merged

fulltimemike avatar Oct 23 '24 21:10 fulltimemike