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

feat(lc/starknet): verify signed commitment proofs from starknet #242

Draft
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

rnbguy
Copy link
Member

@rnbguy rnbguy commented Jan 31, 2025

closes #229

@rnbguy rnbguy force-pushed the rano/starknet-to-cosmos-commitment-proofs branch from cdd28da to 124d5d0 Compare January 31, 2025 11:48
@rnbguy rnbguy force-pushed the rano/starknet-to-cosmos-commitment-proofs branch from 124d5d0 to 07e36a1 Compare January 31, 2025 17:57
@@ -126,7 +158,7 @@ where
},
delay_period,
update_height,
proof_init: counterparty_payload.proof_init.proof_bytes,
proof_init,
Copy link
Member Author

Choose a reason for hiding this comment

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

I successfully managed to sign the proof_init

@@ -126,7 +158,7 @@ where
},
delay_period,
update_height,
proof_init: counterparty_payload.proof_init.proof_bytes,
proof_init,
// TODO(rano): counterparty_payload has empty proofs?
// proof_client: counterparty_payload.proof_client.proof_bytes,
proof_client: vec![0x1],
Copy link
Member Author

Choose a reason for hiding this comment

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

Looks like these two are skipped. Probably ibc-go doesn't use them after cosmos/ibc-go#7129

Comment on lines 249 to 250
// FIXME(rano): generate the signature
proof_ack: dbg!(counterparty_payload.proof_ack.proof_bytes),
Copy link
Member Author

Choose a reason for hiding this comment

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

Currently, the signature verification fails here. But I don't know how to create the signature at this point without access to ConnectionEnd and ConnectionId at starknet.

Copy link
Member Author

Choose a reason for hiding this comment

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

I can query the stored counteryparty connection end (starknet) at cosmos, cosmos's own connection id to recreate the ConnectionEnd.

@rnbguy
Copy link
Member Author

rnbguy commented Jan 31, 2025

To generate the proofs -- specifically signatures, I hacked around. This is not ideal.

To generate a signature, I need

  • starknet consensus root at cosmos (against which the proof is validated)
  • starknet commitment prefix
  • ibc path
  • ibc value

When I am building the message with CosmosChain, I don't have access to StarknetChain. So I can't fetch the consensus_root (block_hash) for starknet.

For now, I pass the block_hash as a dummy proof in the payload -- as I am building the correct proof at message builder anyway.

@soareschen any idea how to do this properly?

@soareschen
Copy link
Collaborator

I think you need to include that in the payload. You can define a custom payload type for Starknet if needed.

@soareschen
Copy link
Collaborator

You could also modify the StarknetCommitmentProof type if it is required in all proofs. I think in Cosmos, the root is part of the MerkleProof struct already.

@rnbguy rnbguy force-pushed the rano/starknet-to-cosmos-commitment-proofs branch from 5828ef2 to 7722d4d Compare February 1, 2025 00:00
@rnbguy
Copy link
Member Author

rnbguy commented Feb 1, 2025

I ended up building the bytes that need to be signed at the Starknet (-to-Cosmos) payload builder step. But Starknet doesn't have access to the Cosmos's default signer. So I signed it at Cosmos's message builder step.

But I don't see how I can sign the bytes for packet commitment, ack and receipt.

@rnbguy
Copy link
Member Author

rnbguy commented Feb 1, 2025

One more question: is the query_packet_receipt supposed to return non_membership_proof too?

async fn query_packet_receipt(
chain: &Chain,
channel_id: &ChannelId,
port_id: &IbcPortId,
sequence: &IbcSequence,
height: &u64,
) -> Result<(Vec<u8>, StarknetCommitmentProof), Chain::Error> {

... for packet_timeout trigger.

@rnbguy
Copy link
Member Author

rnbguy commented Feb 1, 2025

currently fails because the packet_ack proof from Starknet to Cosmos is not valid, i.e. it is not signed by the cosmos default signer.

@rnbguy
Copy link
Member Author

rnbguy commented Feb 1, 2025

Actually, I think, the default_message_signer (or something equivalent) should be used on the Starknet side at payload builder steps.

  • The public key is part of the Starknet light client. It should be part of Starknet's context.
  • We will generate the commitment proofs at payload builder steps directly. The proofs from Starknet should only rely on Starknet's context.

Starknet does have an equivalent to default_message_signer via HasStarknetAccount

pub account: SingleOwnerAccount<Arc<JsonRpcClient<HttpTransport>>, LocalWallet>,

But, it uses ecdsa scheme. But we are using secp256k1 in wasm light clients currently.

Probably, we can use relayer_wallet.signing_key to generate a secp256k1 key pair.

let relayer_wallet = wallets
.get("relayer")
.ok_or_else(|| StarknetBootstrap::raise_error("expect relayer wallet to be present"))?
.clone();

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[lc/starknet] verify membership methods don't have access to CosmWasm context
2 participants