Add support for handling billing process for the service offered by the operator #18

Ayush-Yadav · 2024-01-19T10:23:49Z

Add folllowing functionalities to the serverless backend :-

Interacting with the serverless contract to make sure the deposit corresponding to a tx_hash or code is above a minimum threshold
Tracking the time taken to process the request and generate the response, in order to calculate the service fee to be charged by the operator
Sending transactions to the serverless contract , at regular intervals in the background, for verifying the signature and deducting the fees from the deposits and paying the sum to the operator

vg-27

Operator key can't be used inside enclave. Think, how will it be configured? Using startup script in docker image. And that startup script is included as part of calculating PCRs, implies static contents of enclaves are public, implies operator key is public, which is obviously not possible. Need to think of another way. Also comment if my understanding is wrong. cc: @roshanr95

src/billing_job.rs

src/workerd.rs

Ayush-Yadav · 2024-01-22T12:15:44Z

Operator key can't be used inside enclave. Think, how will it be configured? Using startup script in docker image. And that startup script is included as part of calculating PCRs, implies static contents of enclaves are public, implies operator key is public, which is obviously not possible. Need to think of another way. Also comment if my understanding is wrong. cc: @roshanr95

We'll need the operator's private key in some way to be able to send signed billing transactions using their wallet. One way can be to ask them to store their private key in their system and allow the docker image or the backend to fetch it from there. Not sure if this will be very helpful and can increase the complexity. @roshanr95 Let us discuss this.

src/billing_job.rs

src/handler.rs

src/tests.rs

src/workerd.rs

vg-27

Looks good, except the usage of operator wallet key inside of enclave. So, finalise upon that.

src/billing_job.rs

Ayush-Yadav · 2024-01-29T14:00:35Z

Looks good, except the usage of operator wallet key inside of enclave. So, finalise upon that.

Sure, @roshanr95 is looking into it.

roshanrags · 2024-01-30T04:42:16Z

@Ayush-Yadav Let's pull it outside I guess. There's no easy way to feed the enclave with the operator key and whatever we end up doing is going to be useless once we pull it outside, might as well do it now itself.

roshanrags · 2024-01-30T04:43:05Z

Also, cargo fmt is your friend for all rust formatting stuff, run it before commits or set your editor to run it on save.

vg-27

Rest looks good to me.

src/contract_abi.json

src/workerd.rs

src/billing_handler.rs

src/contracts/billing_contract_abi.json

src/workerd.rs

src/billing_handler.rs

vg-27

What's the design idea behind separating these two calls?

src/billing_handler.rs

Ayush-Yadav · 2024-02-13T09:06:27Z

What's the design idea behind separating these two calls?

There are some additional checks that the user/client need to run on the bill data to ensure that claiming the service fees is actually worth it, given there will be some contract method call cost and balance transfer costs. The client will first fetch the current bill from the enclave, run those checks externally on them and will prepare a list of tx hashes that are worth claiming. They will then hit the 'export' endpoint with those tx hashes and get the final data and the signature required to claim those costs. The client will be then responsible for keeping the data and interacting with the billing contract to claim the bills.

src/billing_handler.rs

vg-27 · 2024-02-13T12:04:03Z

src/billing_handler.rs

+    let signature = hex::encode(rs.to_bytes().append(27 + v.to_byte()).as_slice());
+    let signed_data = hex::encode(signed_data.as_slice());
+
+    HttpResponse::Ok().json(json!({


Is it safe to assume that nonce is supposed to be unique? If yes, then can we save this data with given nonce as key. So that if in a case, client loses the bill, it can query bill again by providing the nonce. And maybe delete the cache after some timeout.

To avoid the case, where client got disconnected from enclave.

Yes nonce is supposed to be unique based on which the contract will catch duplication of bill data. We can store this data in cache for some time but I think once the client receives the data, they are supposed to claim the bill first thing and even if not that they are atleast supposed to store the bill data until they do. I don't know if we should populate the enclave with more data and affect their performance or capacity just to handle this. @roshanr95 Please comment.

Yes, the contract will deduplicate receipts based on the nonce

Storing all, or even some, of them is probably overkill, can maybe just store the very last one that the client can then requery (/billing/latest). Assuming proper persistence (and API usage) is the responsibility of the client, the only real loss vector that remains is network issues while the response is being transmitted, just the last one should be fine to handle this.

We can also protect against duplicate requests by checking the nonce and seeing if it has changed since the last one 🤔

Why is this required inside the enclave? Our major aim is to not allow duplicate requests to the billing contract because that's where the actual transactions happen. If client is sending a duplicate nonce to the enclave then it's their fault because then the enclave will think some balances have been settled when in fact the contract will reject it ,leading to lost bill info. If we were to check nonce inside the enclave then we'll have to check inside the list of all received yet and not just the last one which is probably an overkill.

Agree. Nonce check should also be enforced. Also storing the last bill is sufficient to handle the network issue.

src/billing_handler.rs

vg-27 · 2024-02-16T10:14:36Z

src/billing_handler.rs

-    let mut hash = [0u8; 32];
-    hasher.finalize(&mut hash);
+    let signature = sign_data(bill_claim_data.as_slice(), &appstate.signer).await;
+    if signature.is_err() {


If signature is failing here, then it will fail again when billing/latest will be called. There is no change in parameters or other context data. Signature failing here seems fatal error to me. Double check where it can fail and how developers has suggested to handle those errors.

I looked into the error conditions and possibilities while signing prehash messages and saw if we're using a valid secret scalar key for signing (that I suppose we must be using) and a valid hashing algorithm that gives out a 32-byte hash (which is the case for our Keccak::v256() hasher) then it is very rare that an error would occur. Have added the error checks just in case something weird happens (because of outdated crate version or something).

Btw If we look more closely, there is an aggressive exit happening when serving the requests as well after failing to sign the requests/response data. So even after getting a successful response from the worker serving the app, we're throwing an error response to the user of the app. Is this an acceptable behaviour? @roshanr95 @vg-27

So, if signing is throwing exception then it will be there for every sign call. That means this application is not usable at all in that case, in production.

Then I think, if you just return exception here, that would be sufficient. Any exception due to implementation change, would be hit during testing phase of release.

You can remove the special handling here, it looks like you are trying to recover from error, but it's not. Kinda misleading.

I was just handling it the way it was done when the serverless requests are getting processed by the enclave. I think that is more concerning since it is part of the code already in production. So shouldn't we add an exception here as well:-

pub async fn get_workerd_response(

port: u16, req: HttpRequest, body: actix_web::web::Bytes, signer: &k256::ecdsa::SigningKey, host_header: &str,

) -> Result<HttpResponse, anyhow::Error> {

let mut hasher = Keccak::v256(); hasher.update(b"|oyster-serverless-hasher|"); let timestamp = std::time::SystemTime::now() .duration_since(std::time::UNIX_EPOCH)? .as_secs(); hasher.update(b"|timestamp|"); hasher.update(&timestamp.to_be_bytes()); hasher.update(b"|request|"); hasher.update(b"|method|"); hasher.update(req.method().to_string().as_bytes()); hasher.update(b"|pathandquery|"); hasher.update( req.uri() .path_and_query() .map(|x| x.as_str()) .unwrap_or("") .as_bytes(), ); hasher.update(b"|host|"); hasher.update(host_header.as_bytes()); hasher.update(b"|body|"); hasher.update(&body); let port_str = port.to_string(); let req_url = "http://127.0.0.1:".to_string() + &port_str + "/"; let client = reqwest::Client::new(); let response = req .headers() .into_iter() .fold( client.request(req.method().clone(), req_url), |req, header| req.header(header.0.clone(), header.1.clone()), ) .body(body) .send() .await?; hasher.update(b"|response|"); let mut actix_resp = response.headers().into_iter().fold( HttpResponse::build(response.status()), |mut resp, header| { resp.append_header((header.0.clone(), header.1.clone())); resp }, ); let response_body = response.bytes().await?; hasher.update(b"|body|"); hasher.update(&response_body); let mut hash = [0u8; 32]; hasher.finalize(&mut hash); let (rs, v) = signer.sign_prehash_recoverable(&hash)?;`

Here in this case ? is used to pass on exception. No explicit handling required. Even for your case, I was suggesting just respond back with InternalError. And that would be fine.

If signature is failing here, then it will fail again when billing/latest will be called. There is no change in parameters or other context data. Signature failing here seems fatal error to me. Double check where it can fail and how developers has suggested to handle those errors.

Depends on why it can fail and if it's consistent or not. For example, pretty sure it generates a random number inside, which can probably fail inconsistently.

Storing the last response is mainly just a safeguard I guess, if we don't even want to spend time really figuring out the failure conditions.

Btw If we look more closely, there is an aggressive exit happening when serving the requests as well after failing to sign the requests/response data. So even after getting a successful response from the worker serving the app, we're throwing an error response to the user of the app. Is this an acceptable behaviour? @roshanr95 @vg-27

Yes, responses are insecure without a signature. No point in returning a response if not signed.

For example, pretty sure it generates a random number inside, which can probably fail inconsistently.

Yeah, that's right. Plain ECDSA signature uses random number. But here implementation seems to be using RFC6979, where k is computed deterministically.

Anyways, if there is possibility of inconsistent error. Let's keep the backup of billing data.

src/billing_handler.rs

Ayush-Yadav · 2024-02-13T12:04:14Z

src/billing_handler.rs

+        .sign_prehash_recoverable(&hash)
+        .context("Failed to sign billing data");
+    if sign.is_err() {
+        return HttpResponse::InternalServerError().body(format!("{}", sign.unwrap_err()));


@roshanr95 how to avoid aggressive exit here? by adding a retry mechanism or something?

Ayush-Yadav · 2024-02-13T12:14:25Z

src/billing_handler.rs

+    let signature = hex::encode(rs.to_bytes().append(27 + v.to_byte()).as_slice());
+    let signed_data = hex::encode(signed_data.as_slice());
+
+    HttpResponse::Ok().json(json!({


Yes nonce is supposed to be unique based on which the contract will catch duplication of bill data. We can store this data in cache for some time but I think once the client receives the data, they are supposed to claim the bill first thing and even if not that they are atleast supposed to store the bill data until they do. I don't know if we should populate the enclave with more data and affect their performance or capacity just to handle this. @roshanr95 Please comment.

Ayush-Yadav · 2024-02-13T12:55:12Z

src/billing_handler.rs

+    let signature = hex::encode(rs.to_bytes().append(27 + v.to_byte()).as_slice());
+    let signed_data = hex::encode(signed_data.as_slice());
+
+    HttpResponse::Ok().json(json!({


Why is this required inside the enclave? Our major aim is to not allow duplicate requests to the billing contract because that's where the actual transactions happen. If client is sending a duplicate nonce to the enclave then it's their fault because then the enclave will think some balances have been settled when in fact the contract will reject it ,leading to lost bill info. If we were to check nonce inside the enclave then we'll have to check inside the list of all received yet and not just the last one which is probably an overkill.

src/billing_handler.rs

Ayush-Yadav · 2024-02-16T11:04:04Z

src/billing_handler.rs

-    let mut hash = [0u8; 32];
-    hasher.finalize(&mut hash);
+    let signature = sign_data(bill_claim_data.as_slice(), &appstate.signer).await;
+    if signature.is_err() {


I looked into the error conditions and possibilities while signing prehash messages and saw if we're using a valid secret scalar key for signing (that I suppose we must be using) and a valid hashing algorithm that gives out a 32-byte hash (which is the case for our Keccak::v256() hasher) then it is very rare that an error would occur. Have added the error checks just in case something weird happens (because of outdated crate version or something).

src/billing_handler.rs

Ayush-Yadav · 2024-02-16T11:47:40Z

src/billing_handler.rs

-    let mut hash = [0u8; 32];
-    hasher.finalize(&mut hash);
+    let signature = sign_data(bill_claim_data.as_slice(), &appstate.signer).await;
+    if signature.is_err() {


Btw If we look more closely, there is an aggressive exit happening when serving the requests as well after failing to sign the requests/response data. So even after getting a successful response from the worker serving the app, we're throwing an error response to the user of the app. Is this an acceptable behaviour? @roshanr95 @vg-27

vg-27

~~3 comments are still unresolved.~~

src/billing_handler.rs

vg-27 · 2024-02-19T11:35:51Z

src/billing_handler.rs

-    let mut hash = [0u8; 32];
-    hasher.finalize(&mut hash);
+    let signature = sign_data(bill_claim_data.as_slice(), &appstate.signer).await;
+    if signature.is_err() {


So, if signing is throwing exception then it will be there for every sign call. That means this application is not usable at all in that case, in production.

Then I think, if you just return exception here, that would be sufficient. Any exception due to implementation change, would be hit during testing phase of release.

You can remove the special handling here, it looks like you are trying to recover from error, but it's not. Kinda misleading.

vg-27

LGTM. Let's get approval from Roshan too.

src/billing_handler.rs

Ayush-Yadav requested a review from roshanrags January 19, 2024 10:24

vg-27 requested changes Jan 22, 2024

View reviewed changes

src/billing_job.rs Outdated Show resolved Hide resolved

src/billing_job.rs Outdated Show resolved Hide resolved

src/workerd.rs Outdated Show resolved Hide resolved

Ayush-Yadav requested a review from vg-27 January 24, 2024 08:01

vg-27 requested changes Jan 24, 2024

View reviewed changes

vg-27 reviewed Jan 29, 2024

View reviewed changes

src/billing_job.rs Outdated Show resolved Hide resolved

Ayush-Yadav force-pushed the ayush/billing branch from 1ec904d to 838b64f Compare January 29, 2024 13:56

Ayush-Yadav force-pushed the ayush/billing branch 2 times, most recently from 196716e to 5b1cc2d Compare February 7, 2024 06:12

vg-27 reviewed Feb 7, 2024

View reviewed changes

src/contract_abi.json Outdated Show resolved Hide resolved

Ayush-Yadav requested a review from vg-27 February 9, 2024 05:08

vg-27 reviewed Feb 9, 2024

View reviewed changes

src/workerd.rs Outdated Show resolved Hide resolved

src/workerd.rs Outdated Show resolved Hide resolved

Ayush-Yadav requested a review from vg-27 February 9, 2024 13:04

roshanrags reviewed Feb 12, 2024

View reviewed changes

src/billing_handler.rs Outdated Show resolved Hide resolved

roshanrags reviewed Feb 12, 2024

View reviewed changes

src/contracts/billing_contract_abi.json Outdated Show resolved Hide resolved

vg-27 reviewed Feb 12, 2024

View reviewed changes

src/workerd.rs Outdated Show resolved Hide resolved

src/billing_handler.rs Outdated Show resolved Hide resolved

Ayush-Yadav force-pushed the ayush/billing branch from b9da522 to 7ab6664 Compare February 13, 2024 07:24

roshanrags requested changes Feb 13, 2024

View reviewed changes

src/billing_handler.rs Outdated Show resolved Hide resolved

src/billing_handler.rs Outdated Show resolved Hide resolved

src/billing_handler.rs Outdated Show resolved Hide resolved

src/billing_handler.rs Outdated Show resolved Hide resolved

Ayush-Yadav requested review from roshanrags and vg-27 February 13, 2024 07:59

vg-27 requested changes Feb 13, 2024

View reviewed changes

src/billing_handler.rs Outdated Show resolved Hide resolved

roshanrags reviewed Feb 13, 2024

View reviewed changes

src/billing_handler.rs Outdated Show resolved Hide resolved

vg-27 reviewed Feb 13, 2024

View reviewed changes

Ayush-Yadav requested review from roshanrags and vg-27 February 16, 2024 07:19

vg-27 reviewed Feb 16, 2024

View reviewed changes

src/billing_handler.rs Outdated Show resolved Hide resolved

vg-27 requested changes Feb 16, 2024

View reviewed changes

Ayush-Yadav added 16 commits February 19, 2024 15:07

handle execution based on deposit and store info regarding billing

6f6aefd

add billing scheduler

bab98e6

resolve comments and refactor

2f2fd1e

comments resolve and additional refactoring

d8ecd7f

minor updates

b67ca15

minor fixes

a295980

run server to retrieve bill info

0295563

use abigen to create bindings during compile time

a8a0ee7

renaming

4502fbc

add signature for bill generated

2ef0b87

resolve comments and refactor code

350d1cf

updating billing info

bd80202

create endpoints to retrieve and sign bill

409d90f

resolve comments

1a47140

retrieve latest bill info

d59dc0c

resolve comments

d317e86

Ayush-Yadav force-pushed the ayush/billing branch from 2a763f5 to d317e86 Compare February 19, 2024 09:42

Ayush-Yadav requested a review from vg-27 February 19, 2024 09:43

Ayush-Yadav commented Feb 19, 2024

View reviewed changes

vg-27 reviewed Feb 19, 2024

View reviewed changes

resolve comments

f21eee6

Ayush-Yadav requested a review from vg-27 February 20, 2024 07:42

vg-27 approved these changes Feb 20, 2024

View reviewed changes

Ayush-Yadav added 2 commits February 22, 2024 12:26

refactor using let-else

66703c9

refactor and optimize

42a3d4c

roshanrags reviewed Feb 22, 2024

View reviewed changes

src/billing_handler.rs Outdated Show resolved Hide resolved

src/billing_handler.rs Outdated Show resolved Hide resolved

resolve comments

b4f8a3a

Ayush-Yadav requested a review from roshanrags February 22, 2024 12:41

use try_connect to make rpc connection

6fe2e1b

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for handling billing process for the service offered by the operator #18

Add support for handling billing process for the service offered by the operator #18

Ayush-Yadav commented Jan 19, 2024

vg-27 left a comment

Ayush-Yadav commented Jan 22, 2024

vg-27 left a comment

Ayush-Yadav commented Jan 29, 2024

roshanrags commented Jan 30, 2024

roshanrags commented Jan 30, 2024 •

edited

Loading

vg-27 left a comment

vg-27 left a comment

Ayush-Yadav commented Feb 13, 2024

vg-27 Feb 13, 2024

Ayush-Yadav Feb 13, 2024

roshanrags Feb 13, 2024

roshanrags Feb 13, 2024

Ayush-Yadav Feb 13, 2024

vg-27 Feb 14, 2024

vg-27 Feb 16, 2024

Ayush-Yadav Feb 16, 2024

Ayush-Yadav Feb 16, 2024

vg-27 Feb 19, 2024

Ayush-Yadav Feb 19, 2024 •

edited

Loading

vg-27 Feb 20, 2024

roshanrags Feb 20, 2024 •

edited

Loading

roshanrags Feb 20, 2024 •

edited

Loading

vg-27 Feb 20, 2024

Ayush-Yadav Feb 13, 2024

Ayush-Yadav Feb 13, 2024

Ayush-Yadav Feb 13, 2024

Ayush-Yadav Feb 16, 2024

Ayush-Yadav Feb 16, 2024

vg-27 left a comment •

edited

Loading

vg-27 Feb 19, 2024

vg-27 left a comment

Add support for handling billing process for the service offered by the operator #18

Are you sure you want to change the base?

Add support for handling billing process for the service offered by the operator #18

Conversation

Ayush-Yadav commented Jan 19, 2024

vg-27 left a comment

Choose a reason for hiding this comment

Ayush-Yadav commented Jan 22, 2024

vg-27 left a comment

Choose a reason for hiding this comment

Ayush-Yadav commented Jan 29, 2024

roshanrags commented Jan 30, 2024

roshanrags commented Jan 30, 2024 • edited Loading

vg-27 left a comment

Choose a reason for hiding this comment

vg-27 left a comment

Choose a reason for hiding this comment

Ayush-Yadav commented Feb 13, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ayush-Yadav Feb 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

roshanrags Feb 20, 2024 • edited Loading

Choose a reason for hiding this comment

roshanrags Feb 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vg-27 left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vg-27 left a comment

Choose a reason for hiding this comment

roshanrags commented Jan 30, 2024 •

edited

Loading

Ayush-Yadav Feb 19, 2024 •

edited

Loading

roshanrags Feb 20, 2024 •

edited

Loading

roshanrags Feb 20, 2024 •

edited

Loading

vg-27 left a comment •

edited

Loading