-
Notifications
You must be signed in to change notification settings - Fork 0
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: implement Pairing API #3
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Huge work! First review iteration from my side.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Next review iteration!
pairing_api/src/pairing.rs
Outdated
/// Pairing Delete error code. | ||
const PAIRING_DELETE_ERROR_CODE: i64 = 6000; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something we need to do when writing doc comments is not to add unnecessary ones, this is the same as the const name and not needed. If a comment doesn't explain something not obvious then it's not needed. For pub items we need doc comments, so we should try to write something that makes it clearer to the user of the sdk.
@borngraced ps signature tests started to fail |
Btw noticed WalletConnectRs is actively using alloy with foundry(forge) in their project, a good example to learn from (as we should move to alloy in kdf, instead of using old web3 crate) |
AFAIK, only CI is failing.. I will check ✔️ |
I have this locally too, run this command
|
just need to add let mut args = vec![
"create",
"--contracts=relay_rpc/contracts",
contract_name,
"--rpc-url",
rpc_url.as_str(),
"--private-key",
&key_encoded,
"--cache-path",
&cache_folder,
"--out",
&out_folder,
"--broadcast",
]; UPD: strange that Smith (author of the code) didnt add --broadcast flag (it submits the transaction to the blockchain specified by the --rpc-url argument), it is safe to add this flag as they use #[tokio::test]
async fn test_eip1271_pass() {
let (_anvil, rpc_url, private_key) = spawn_anvil().await;
let contract_address = deploy_contract(
&rpc_url,
&private_key,
EIP1271_MOCK_CONTRACT,
Some(&Address::from_private_key(&private_key).to_string()),
)
.await;
// the rest of the code
}
pub async fn spawn_anvil() -> (AnvilInstance, Url, SigningKey) {
let anvil = Anvil::at(format_foundry_dir("bin/anvil")).spawn();
let provider = anvil.endpoint().parse().unwrap();
let private_key = anvil.keys().first().unwrap().clone();
(
anvil,
provider,
SigningKey::from_bytes(&private_key.to_bytes()).unwrap(),
)
} |
I wonder how this has suddenly become a problem perhaps an update has happened somewhere in CI. fix anyways.. thanks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
This PR implements the WalletConnect Pairing API, enabling secure communication between dApps and wallets. It manages pairing requests, responses, and lifecycle events (creating, deleting, extending, and pinging pairings).
Key features:
Test overview:
The test demonstrates the full lifecycle of a WalletConnect pairing, from establishment to termination, using the pairing_api and relay_client crates.
https://specs.walletconnect.com/2.0/specs/clients/core/pairing/
https://specs.walletconnect.com/2.0/specs/clients/core/crypto/crypto-keys
https://specs.walletconnect.com/2.0/specs/clients/core/pairing/data-structures
https://specs.walletconnect.com/2.0/specs/clients/core/pairing/pairing-api