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

Devx improvements to WebSocketListener framework #208

Open
dangush opened this issue Sep 17, 2024 · 2 comments
Open

Devx improvements to WebSocketListener framework #208

dangush opened this issue Sep 17, 2024 · 2 comments
Labels
architecture High-level architectural concerns quartz-cli

Comments

@dangush
Copy link
Contributor

dangush commented Sep 17, 2024

Summary

Along with the contract and enclave binary, devs must also implement the WebSocketListener trait which defines a script which will listen to tendermint events from the configured node_url and call the enclave endpoints based on these events.

Currently, there's no predefined functions or structure to how this is implemented, making it quite unintuitive for a quartz dev. With this issue I'd like to make some architecture decisions around pieces we can provide as part of the framework to help devs.

Issues

  1. Devs must implement their own logic for indexing into the Tendermint event to determine if their WebSocketListener handler should trigger the core logic.

  2. Devs must directly call the tm-prover library's prove function, which requires filling a large configuration parameter struct and blocking execution for the length of 1-2 TM blocks.

  • Since the websocketlistener handlers are called sequentially on events coming in, if the dev wants to call the enclave on every block, this means an infinitely-growing delay if the handler must wait 1-2 blocks in order to finish executing logic
  1. Devs need to add this code somewhere:
impl<A: Attestor> IntoServer for TransfersService<A> {
    type Server = SettlementServer<TransfersService<A>>;

    fn into_server(self) -> Self::Server {
        SettlementServer::new(self)
    }
}

which we could simplify with a macro quite easily.

  1. There are 3 different objects which effectively represent the same data, which becomes confusing for the dev.
    a. There's the protobuf message object, which is a struct containing a "message" field. it's named based on the .proto file
    b. There's the object that the protobuf message actually contains in serialized form. One is essentially the wire type and the other the domain type.
    c. There's the struct that represents the message sent to the smart contract. It usually contains the exact same info as what is returned by the enclave

So, for example, when getting data returned by the enclave and sending it to the smart contract, the wire type, domain type that you must serialize it into, and the object that you then send to the smart contract are all differently named objects containing the same data.

Discussion

1

For # 1 , I'm trying to think of how I can make the experience more "fill in the blank" in a way which is more helpful than restrictive. In the watchexec library, devs simply pass a filename to the watchexec config, which will then send a 'file update' event over a tokio channel which contains all the metadata about the dev's requested file upon detection of changes. In this case, the dev is starting with the tendermint event. However, this tendermint event needs to be evaluated for whether it matches the dev's query.

Additionally, there is some redundancy in the code now where the dev loops over an event to determine whether to react before looping again to collect desired information from attributes, etc:

fn is_transfer_event(event: &Event) -> bool {
// Check if the event is a transaction type
if let Some(EventType::Tx) = event.event_type() {
// Check for the "wasm.action" key with the value "init_clearing"
if let Some(events) = &event.events {
return events.iter().any(|(key, _)| key == "wasm-transfer.action");
}
}
false
}

let mut sender = None;
let mut contract_address = None;
let mut emphemeral_pubkey = None;
if let Some(events) = &event.events {
for (key, values) in events {
match key.as_str() {
"message.sender" => {
sender = values.first().cloned();
}
"execute._contract_address" => {
contract_address = values.first().cloned();
}
"wasm.query_balance.emphemeral_pubkey" => {
// TODO: fix typo
emphemeral_pubkey = values.first().cloned();
}
_ => {}
}
}
}

I'm thinking that we should define a trait with a method like

fn match_and_extract(params: Vec<&str>) -> Option<Vec<Event>>> {}

which combines these two loops. If the return type is None, the event should be ignored. Otherwise (if there's a match), the return vector contains the specified events & their attributes. The Event obj is this one.

2

The tm-prover is called by both the handshake command in the quartz cli and by devs for WebSocketListener impls. It would be good to get rid of this repetitive logic by moving it into a library somewhere. The issue is that the arguments for the prove function are mostly values that are supplied by the quartz.toml config file. So pulling this prover calling logic into a library (such as quartz-common) would mean that this library would have to be aware of the file structure of a quartz app, or the .config folder more specifically. Alternatively, the quartz-cli would be imported as a library and a function for the prover could be used.

I'm not sure what the optimal solution here is.

3

I'll add some possible implementations here later.

@dangush dangush added quartz-cli architecture High-level architectural concerns labels Sep 17, 2024
@hu55a1n1
Copy link
Member

Great points! For 3, I suggest something like ->

#[derive(Clone, Debug, IntoServer(TransfersServer))]
struct TransfersService<A>{ 
    /* ... */ 
}

@dangush
Copy link
Contributor Author

dangush commented Sep 17, 2024

Copying reply to comment from Shoaib in #174

Some of the serialization / deserialization stuff could def be hidden into helper functions

    // Build request
    let update_contents = QueryRequestMessage {
        state,
        address: Addr::unchecked(&msg_sender), // sender comes from TX event, therefore is checked
        ephemeral_pubkey: HexBinary::from_hex(&pubkey)?,
    };

    // Send QueryRequestMessage to enclave over tonic gRPC client
    let request = Request::new(QueryRequest {
        message: json!(update_contents).to_string(),
    });
    // Extract json from the enclave response
    let attested: RawAttested<QueryResponseMsg, Vec<u8>> =
        serde_json::from_str(&query_response.message)
            .map_err(|e| anyhow!("Error deserializing QueryResponseMsg from enclave: {}", e))?;

    // Build on-chain response
    // TODO add non-mock support
    let setoffs_msg = ExecuteMsg::QueryResponse::<RawMockAttestation>(AttestedMsg {
        msg: RawAttestedMsgSansHandler(attested.msg),
        attestation: MockAttestation(attested.attestation.try_into().unwrap()).into(),
    });

The helpers could be part of quartz-enclave I guess

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
architecture High-level architectural concerns quartz-cli
Projects
None yet
Development

No branches or pull requests

2 participants