Skip to content

Commit

Permalink
fix: grpc inconsistent serialization of keys (see issue #4224) (#4491)
Browse files Browse the repository at this point in the history
Description
--- 
We address inconsistent serialization of keys across base node and wallet grpc servers. The first uses a little endian 32-byte representation of the keys, whereas the second a 64-byte hex representation. We opt to use a hex-representation for the public key on the wallet grpc side.

Motivation and Context
--- 
Fixes #4224.

How Has This Been Tested?
--- 
Manually.
  • Loading branch information
jorgeantonio21 authored Aug 22, 2022
1 parent ea74603 commit bdb262c
Show file tree
Hide file tree
Showing 6 changed files with 98 additions and 58 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -198,9 +198,9 @@ impl wallet_server::Wallet for WalletGrpcServer {
async fn identify(&self, _: Request<GetIdentityRequest>) -> Result<Response<GetIdentityResponse>, Status> {
let identity = self.wallet.comms.node_identity();
Ok(Response::new(GetIdentityResponse {
public_key: identity.public_key().to_string().into_bytes(),
public_key: identity.public_key().to_vec(),
public_address: identity.public_address().to_string(),
node_id: identity.node_id().to_string().into_bytes(),
node_id: identity.node_id().to_vec(),
}))
}

Expand Down
5 changes: 3 additions & 2 deletions integration_tests/features/support/ffi_steps.js
Original file line number Diff line number Diff line change
Expand Up @@ -646,7 +646,7 @@ Then("I want to get public key of ffi wallet {word}", function (name) {
let wallet = this.getWallet(name);
let public_key = wallet.identify();
expect(public_key.length).to.be.equal(
64,
32,
`Public key has wrong length : ${public_key}`
);
});
Expand Down Expand Up @@ -723,7 +723,8 @@ Then(
() => {
let publicKeys = wallet.listConnectedPublicKeys();
return (
publicKeys && publicKeys.some((p) => p === nodeIdentity.public_key)
publicKeys &&
publicKeys.some((p) => p === nodeIdentity.public_key.toString("hex"))
);
},
true,
Expand Down
10 changes: 8 additions & 2 deletions integration_tests/features/support/steps.js
Original file line number Diff line number Diff line change
Expand Up @@ -508,7 +508,9 @@ When(

await waitForPredicate(async () => {
let peers = await firstNodeClient.listConnectedPeers();
return peers.some((p) => secondNodeIdentity.public_key === p.public_key);
return peers.some(
(p) => secondNodeIdentity.public_key.toString("hex") === p.public_key
);
}, 50 * 1000);
}
);
Expand All @@ -519,7 +521,11 @@ Then(/(.*) is connected to (.*)/, async function (firstNode, secondNode) {
const secondNodeIdentity = await secondNodeClient.identify();
let peers = await firstNodeClient.listConnectedPeers();
expect(
peers.some((p) => secondNodeIdentity.public_key === p.public_key)
peers.some(
(p) =>
secondNodeIdentity.public_key.toString("hex") ===
p.public_key.toString("hex")
)
).to.be.true;
});

Expand Down
Loading

0 comments on commit bdb262c

Please sign in to comment.