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

grant_installation #50

Merged
merged 4 commits into from
Jan 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions gateway-types/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,24 @@ pub struct Message {
/// Signature of S
pub s: Vec<u8>,
}

/// GrantInstallationResult represents the result of a grant installation operation in the DID registry.
///
/// This struct encapsulates the outcome of an attempt to grant an installation,
/// providing details about the operation's status, a descriptive message, and the
/// transaction identifier associated with the blockchain transaction.
///
/// # Fields
/// * `status` - A `String` indicating the outcome status of the operation. Typically, this
/// would be values like "Success" or "Failure".
/// * `message` - A `String` providing more detailed information about the operation. This
/// can be a success message, error description, or any other relevant information.
/// * `transaction` - A `String` representing the unique identifier of the transaction on the
/// blockchain. This can be used to track the transaction in a blockchain explorer.
///
#[derive(Serialize, Deserialize, Clone)]
tsachiherman marked this conversation as resolved.
Show resolved Hide resolved
pub struct GrantInstallationResult {
pub status: String,
pub message: String,
pub transaction: String,
}
52 changes: 47 additions & 5 deletions registry/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,15 @@

use std::str::FromStr;

use error::ContactOperationError;
use ethers::types::{H160, U256};
use ethers::{core::types::Signature, providers::Middleware, types::Address};
use gateway_types::GrantInstallationResult;
use lib_didethresolver::{
did_registry::DIDRegistry,
types::{Attribute, XmtpAttribute},
};

use error::ContactOperationError;

pub struct ContactOperations<Middleware> {
registry: DIDRegistry<Middleware>,
}
Expand All @@ -23,16 +24,57 @@
Self { registry }
}

fn resolve_did_address(&self, did: String) -> Result<H160, ContactOperationError<M>> {
// for now, we will just assume the DID is a valid ethereum wallet address
// TODO: Parse or resolve the actual DID
Copy link
Contributor

Choose a reason for hiding this comment

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

Made into an issue, #51

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// TODO: Parse or resolve the actual DID

let address = Address::from_str(&did)?;
Ok(address)
}

pub async fn grant_installation(
&self,
did: String,
name: XmtpAttribute,
value: Vec<u8>,
signature: Signature,
validity: U256,
) -> Result<GrantInstallationResult, ContactOperationError<M>> {
let address = self.resolve_did_address(did)?;
let attribute: [u8; 32] = Attribute::from(name).into();
log::debug!(
"setting attribute {:#?}",
String::from_utf8_lossy(&attribute)

Check warning on line 46 in registry/src/lib.rs

View check run for this annotation

Codecov / codecov/patch

registry/src/lib.rs#L45-L46

Added lines #L45 - L46 were not covered by tests
Copy link
Contributor

Choose a reason for hiding this comment

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

Remember that this string will always be created, even if this log is suppressed. This may be slightly too verbose logging.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm perfectly fine eliminating that. it was used during my debugging, but I don't feel strongly here.

);

let transaction_receipt = self
.registry
.set_attribute_signed(
address,
signature.v.try_into()?,
signature.r.into(),
signature.s.into(),
attribute,
value.into(),
validity,
)
.send()
.await?
.await?;
Copy link
Contributor

Choose a reason for hiding this comment

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

so far, we've been doing this strategy with .await?.await? which first sends the transaction, gets back a PendingTransaction, and then waits for it to be included + a specified amount of confirmations (default of 1). I like what you did below, though, but unsure what the better path would be:

Do we make the gateway wait until tx is confirmed or do we just return the tx_hash and have libxmtp/client wait?

We can instead just return the txHash from the RPC, and not wait on transaction inclusion, leaving tx confirmations up to the client. We do give up some control if something goes wrong.

Not a super important detail, but maybe worth a quick discussion WDYT @tsachiherman @jac18281828 @37ng

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that it's a very important observation, as this is a blocking operation that consumes servers memory for the entire duration of the operation.
Another point is that regardless if we make it "non-blocking" or not, we need to be able to detect a fail cases so that we can log these.

Copy link
Contributor

@insipx insipx Jan 29, 2024

Choose a reason for hiding this comment

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

That's true, IMO I think we should hand off control of the transaction to the client and just return txHash. A PendingTransaction can be easily rebuilt from libxmtp side by just providing the hash and a provider, so if anything fails they could re-submit the transaction

We could also provide an endpoint to do this for them, i.e subscribeTransaction or something along those lines

Copy link
Contributor

Choose a reason for hiding this comment

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

We should avoid waiting whenever possible in the service. We haven't spent much time optimizing the number of threads, etc., these threads will be precious.

Copy link
Contributor

Choose a reason for hiding this comment

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

made an issue for this: #52

the return of revoke_installation will have to change as well

Ok(GrantInstallationResult {
status: "completed".to_string(),
message: "Installation request complete.".to_string(),
transaction: transaction_receipt.unwrap().transaction_hash.to_string(),
})
}

pub async fn revoke_installation(
&self,
did: String,
name: XmtpAttribute,
value: Vec<u8>,
signature: Signature,
) -> Result<(), ContactOperationError<M>> {
// for now, we will just assume the DID is a valid ethereum wallet address
// TODO: Parse or resolve the actual DID
let address = Address::from_str(&did)?;
let address = self.resolve_did_address(did)?;
let attribute: [u8; 32] = Attribute::from(name).into();
log::debug!(
"Revoking attribute {:#?}",
Expand Down
98 changes: 98 additions & 0 deletions xps-gateway/src/rpc/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use ethers::core::types::Signature;
use ethers::prelude::*;
use jsonrpsee::{proc_macros::rpc, types::ErrorObjectOwned};

use gateway_types::GrantInstallationResult;
use gateway_types::Message;
use lib_didethresolver::types::XmtpAttribute;

Expand All @@ -14,6 +15,103 @@ pub trait Xps {
#[method(name = "sendMessage")]
async fn send_message(&self, _message: Message) -> Result<(), ErrorObjectOwned>;

/// # Documentation for JSON RPC Endpoint: `grantInstallation`
///
/// ## Overview
///
/// The `grantInstallation` method is used to register an installation on the network and associate the installation with a concrete identity.
///
/// ## JSON RPC Endpoint Specification
///
/// ### Method Name
/// `grantInstallation`
///
/// ### Request Parameters
/// did: string
/// name: String,
/// value: String,
/// signature: Signature,
///
/// ### Request Format
/// ```json
/// {
/// "jsonrpc": "2.0",
/// "method": "status",
/// "id": 1
/// }
/// ```

/// - `jsonrpc`: Specifies the version of the JSON RPC protocol being used. Always "2.0".
/// - `method`: The name of the method being called. Here it is "grantInstallation".
/// - `id`: A unique identifier established by the client that must be number or string. Used for correlating the response with the request.

/// ### Response Format
/// The response will typically include the result of the operation or an error if the operation was unsuccessful.

/// #### Success Response
/// ```json
/// {
/// "jsonrpc": "2.0",
/// "result": "OK",
/// "id": 1
/// }
/// ```
///
/// - `result`: Contains data related to the success of the operation. The nature of this data can vary based on the implementation.
///
/// #### Error Response
/// ```json
/// {
/// "jsonrpc": "2.0",
/// "error": {
/// "code": <error_code>,
/// "message": "<error_message>"
/// },
/// "id": 1
/// }
/// ```
///
/// - `error`: An object containing details about the error.
/// - `code`: A numeric error code.
/// - `message`: A human-readable string describing the error.
///
/// ### Example Usage
///
/// #### Request
/// ```json
/// {
/// "jsonrpc": "2.0",
/// "method": "status",
/// "id": 42
/// }
/// ```
///
/// #### Response
/// ```json
/// {
/// "jsonrpc": "2.0",
/// "result": "OK",
/// "id": 42
/// }
/// ```
///
/// ### Command Line Example
/// ```bash
/// $ $ curl -H "Content-Type: application/json" -d '{"id":7000, "jsonrpc":"2.0", "method":"xps_status"}' http:///localhost:34695
/// {"jsonrpc":"2.0","result":"OK","id":7000}
/// ```
///
/// ### Notes
/// - The system should have proper error handling to deal with invalid requests, unauthorized access, and other potential issues.
#[method(name = "grantInstallation")]
async fn grant_installation(
&self,
did: String,
name: XmtpAttribute,
value: Vec<u8>,
signature: Signature,
) -> Result<GrantInstallationResult, ErrorObjectOwned>;

/// # Documentation for JSON RPC Endpoint: `revoke_installation`
///
/// ## JSON RPC Endpoint Specification
Expand Down
25 changes: 25 additions & 0 deletions xps-gateway/src/rpc/methods.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,11 @@ use jsonrpsee::types::error::ErrorCode;
use async_trait::async_trait;
use ethers::prelude::*;
use ethers::{core::types::Signature, providers::Middleware};
use gateway_types::GrantInstallationResult;
use jsonrpsee::types::ErrorObjectOwned;
use lib_didethresolver::types::XmtpAttribute;
use rand::{rngs::StdRng, SeedableRng};
use std::sync::Arc;
use thiserror::Error;

use gateway_types::Message;
Expand All @@ -20,13 +22,15 @@ use registry::{error::ContactOperationError, ContactOperations};
pub struct XpsMethods<P: Middleware + 'static> {
contact_operations: ContactOperations<GatewaySigner<P>>,
pub wallet: LocalWallet,
pub signer: Arc<GatewaySigner<P>>,
}

impl<P: Middleware> XpsMethods<P> {
pub fn new(context: &GatewayContext<P>) -> Self {
Self {
contact_operations: ContactOperations::new(context.registry.clone()),
wallet: LocalWallet::new(&mut StdRng::from_entropy()),
signer: context.signer.clone(),
}
}
}
Expand All @@ -44,6 +48,27 @@ impl<P: Middleware + 'static> XpsServer for XpsMethods<P> {
Ok("OK".to_string())
}

async fn grant_installation(
&self,
did: String,
name: XmtpAttribute,
value: Vec<u8>,
signature: Signature,
) -> Result<GrantInstallationResult, ErrorObjectOwned> {
log::debug!("xps_grantInstallation called");
let block_number = self.signer.get_block_number().await.unwrap();
let validity_period: U64 = U64::from(60 * 60 * 24 * 365 / 5); // number of round in one year, assuming 5-second round.
let validity = block_number + validity_period;

let result = self
.contact_operations
.grant_installation(did, name, value, signature, U256::from(validity.as_u64()))
.await
.map_err(RpcError::from)?;

Ok(result)
}

async fn revoke_installation(
&self,
did: String,
Expand Down
66 changes: 65 additions & 1 deletion xps-gateway/tests/integration_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ use lib_didethresolver::{
};
use xps_gateway::rpc::XpsClient;

use ethers::types::{Address, U256};
use ethers::middleware::Middleware;
use ethers::types::{Address, U256, U64};
use gateway_types::Message;

use integration_util::*;
Expand Down Expand Up @@ -51,6 +52,69 @@ async fn test_wallet_address() -> Result<(), Error> {
.await
}

#[tokio::test]
async fn test_grant_installation() -> Result<(), Error> {
with_xps_client(None, |client, context, resolver, anvil| async move {
let wallet: LocalWallet = anvil.keys()[3].clone().into();
let me = get_user(&anvil, 3).await;
let name = *b"xmtp/installation/hex ";
let value = b"02b97c30de767f084ce3080168ee293053ba33b235d7116a3263d29f1450936b71";

let attribute = XmtpAttribute {
purpose: XmtpKeyPurpose::Installation,
encoding: KeyEncoding::Hex,
};

let block_number = context.signer.get_block_number().await.unwrap();
let validity_period: U64 = U64::from(60 * 60 * 24 * 365 / 5); // number of round in one year, assuming 5-second round.
let validity = block_number + validity_period;

let signature = wallet
.sign_attribute(
&context.registry,
name,
value.to_vec(),
U256::from(validity.as_u64()),
)
.await?;

client
.grant_installation(
format!("0x{}", hex::encode(me.address())),
attribute,
value.to_vec(),
signature,
)
.await?;

let doc = resolver
.resolve_did(me.address(), None)
.await
.unwrap()
.document;

assert_eq!(doc.verification_method.len(), 2);
assert_eq!(
doc.verification_method[0].id,
DidUrl::parse(format!(
"did:ethr:0x{}#controller",
hex::encode(me.address())
))
.unwrap()
);
assert_eq!(
doc.verification_method[1].id,
DidUrl::parse(format!(
"did:ethr:0x{}?meta=installation#xmtp-0",
hex::encode(me.address())
))
.unwrap()
);
Ok(())
})
.await
}

#[tokio::test]
async fn test_revoke_installation() -> Result<(), Error> {
with_xps_client(None, |client, context, resolver, anvil| async move {
Expand Down
Loading