[Faucet] Adding new API with old function call for fallback #12648
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 6 Ignored Deployments
|
crates/sui-faucet/src/main.rs
Outdated
| @@ -92,6 +92,7 @@ async fn main() -> Result<(), anyhow::Error> { | |||
| .route("/gas", post(request_gas)) | |||
| .route("/v1/gas", post(batch_request_gas)) | |||
| .route("/v1/status", post(request_status)) | |||
| .route("/v1/gas_old", post(request_gas_v1_interface)) | |||
There was a problem hiding this comment.
The idea of the fallback is that it's controlled by a config flag on faucet, so we shouldn't need a new endpoint, instead, based on a config flag, /v1/gas would start serving batch requests without actually batching them. This way, if there's an issue with the new implementation, we can fall back to the old one without having to tell any client to change which endpoitn they are calling.
There was a problem hiding this comment.
Changed this structure.
crates/sui-faucet/src/main.rs
Outdated
| match result { | ||
| Ok(_) => { | ||
| info!(uuid =?id, "Request is successfully served"); | ||
| (StatusCode::CREATED, Json(BatchFaucetResponse::from(id))) |
There was a problem hiding this comment.
The response structure should be exactly the same as for batch_request_gas, i.e. it says that the request has been accepted, when in fact it has already successfully served the response.
There was a problem hiding this comment.
This is the same because I just send back the UUID which is what batch_request_gas sends back
crates/sui-faucet/src/main.rs
Outdated
| match result { | ||
| Ok(_) => { | ||
| info!(uuid =?id, "Request is successfully served"); | ||
| (StatusCode::CREATED, Json(BatchFaucetResponse::from(id))) |
There was a problem hiding this comment.
Continuing the thread that was previously here -- the reason the response is not the same is because the StatusCode here is CREATED rather than ACCEPTED above. It looks like we should be able to fix that by sharing most of the implementation between the two parts (see below).
Note, I also added a logging message for the fallback path, so we can easily check that things are working by enable informational logs, when we first start the faucet with batching disabled.
let FaucetRequest::FixedAmountRequest(request) = payload else {
return (
StatusCode::BAD_REQUEST,
Json(BatchFaucetResponse::from(FaucetError::Internal(
"Input Error.".to_string(),
))),
)
};
// TODO (jian): remove this feature gate when batch has proven to be baked long enough
let result = if state.config.batch_enabled {
// TODO (jian): get rid of this task spawning since recycled coins are now handled by the batching task.
spawn_monitored_task!(async move {
state
.faucet
.batch_send(
id,
requests.recipient,
&vec![state.config.amount; state.config.num_coins],
)
.await
})
.await
.unwrap()
} else {
info!(uuid = ?id, "Falling back to v1 implementation");
spawn_monitored_task!(async move {
state
.faucet
.send(
id,
requests.recipient,
&vec![state.config.amount; state.config.num_coins],
)
.await
})
.await
.unwrap()
}
match result {
Ok(_) => {
info!(uuid = ?id, "Request is successfully served");
(StatusCode::CREATED, Json(BatchFaucetResponse::from(id))
},
Err(e) => {
warn!(uuid = ?id, "Failed to request gas: {:?}", v);
(
StatusCode::INTERNAL_SERVER_ERROR,
Json(BatchFaucetResponse::from(e)),
)
}
}
## Description Addition of a feature flag to gate the batch api behavior ## Test Plan Unit testing to make sure the send UUID can be found. --- If your changes are not user-facing and not a breaking change, you can skip the following section. Otherwise, please indicate what changed, and then add to the Release Notes section as highlighted during the release process. ### Type of Change (Check all that apply) - [ ] protocol change - [x] user-visible impact - [ ] breaking change for a client SDKs - [ ] breaking change for FNs (FN binary must upgrade) - [ ] breaking change for validators or node operators (must upgrade binaries) - [ ] breaking change for on-chain data layout - [ ] necessitate either a data wipe or data migration ### Release notes
Description
Addition of a feature flag to gate the batch api behavior
Test Plan
Unit testing to make sure the send UUID can be found.
If your changes are not user-facing and not a breaking change, you can skip the following section. Otherwise, please indicate what changed, and then add to the Release Notes section as highlighted during the release process.
Type of Change (Check all that apply)
Release notes