chore: add sdk::PublicKey to workspaces::PublicKey conversion

[ChaoticTempest]

Resolves #262

This adds a TryFrom<sdk::PublicKey> impl to convert sdk::PublicKey into workspaces::PublicKeys, along with a test to check serialization. This also has a new feature flag interop_sdk just in a case they don't want to add a big dependency to their testing pipeline. It's enabled by default anyways

Borsh serialization had to change from the previous PR #265 to make it compatible to be sent to a contract through borsh serialization. The new format is now [buffer_len, key_type, key_data..]

[miraclx]

Just took a look at sdk::PublicKey's borsh-serialization format, and it's really weird tbh, I think in general it's not ideal to assume byte slices deserialize the same way for two types in differing crates with the same name. We should be able to retain the current behavior, and instead just provide two-way conversion at the API-level.

So, if you have a byte slice that successfully deserializes into sdk::PublicKey, you can just call into() on that instead of attempting to deserialize the same byte slice into a workspaces::PublicKey. And vice-versa.

[ChaoticTempest]

Just took a look at sdk::PublicKey's borsh-serialization format, and it's really weird tbh, I think in general it's not ideal to assume byte slices deserialize the same way for two types in differing crates with the same name. We should be able to retain the current behavior, and instead just provide two-way conversion at the API-level.

So I was trying to eliminate a friction point with this, but historically for serialized types in workspaces, it's just been passing them into args* functions without any sort of conversions into sdk types like so:

contract.call("function")
    .args_json(json!({
         "account_id": AccountId("user.test"),
    })
    .transact().await?;

Adding the conversion would feel pretty inconsistent:

contract.call("function")
    .args_json(json!({
         "account_id": AccountId("user.test"),
         "public_key": sdk::PublicKey::from(PublicKey::try_from_bytes(&bytes)?)
    })
    .transact().await?;

since we'd now need to know what types are serializable via workspaces or sdk. I'd like not to go down that route, since the experience wouldn't be nice I'd say.

So, if you have a byte slice that successfully deserializes into sdk::PublicKey, you can just call into() on that instead of attempting to deserialize the same byte slice into a workspaces::PublicKey. And vice-versa.

Like creating sdk::PublicKey won't have an issue if you have the byte slice, but it's a separate issue when workspaces would give you workspaces::PublicKeys from its APIs, which isn't ideal cause of the inconsistency with serializing them later mentioned above.

[miraclx]

but it's a separate issue when workspaces would give you workspaces::PublicKeys from its APIs, which isn't ideal cause of the inconsistency with serializing them later mentioned above.

That's a fair point, although just noting that serde-serialization looks to be consistent between sdk/workspaces' PublicKeys. But I understand the inclination to implicitly pass one type for another, especially since there's no strictly typed checks in that context. Inconsistent borsh formats moves into a runtime issue. Could definitely save a lot of hair pulling.

[ChaoticTempest]

@frol this should be good to go whenever you want to fully take a look at it

[frol]

I am not exactly sure about having interop_sdk to be enabled by default, but I think in the bright future where PublicKey is extracted into a separate crate and shared across nearcore, near-sdk-rs, and workspace-rs, we can drop this code altogether and the feature will become no-op

[ChaoticTempest]

@frol by enabling it by default, it offers the least friction to testing contracts which workspaces should mostly be geared towards. It shouldn't be too huge of a concern since this is during testing time. Where it becomes bloatware is when workspaces gets used for automation or as a near-api-rs replacement, but I don't think that's a huge concern either since people can disable the feature if it becomes too bothersome.

[...] but I think in the bright future where PublicKey is extracted into a separate crate and shared across nearcore, near-sdk-rs, and workspace-rs, we can drop this code altogether and the feature will become no-op

So, the PublicKey in near-sdk-rs isn't exactly the same as the one used in near-primitives. So there's the extra requirement that PublicKey is the same and consistent between all three libraries. I fear the near_sdk::PublicKey would be harder to make consistent with the near_primitives one as that one has stricter requirements like being wasm-compilable. The alternative I went for here is to just keep the byte format the same between the two