Re: serde and backends

[tpmccallum]

Hi, The Cargo.toml specifies u64_backend for default features and u32_backend for the wasm feature.

When adding a new feature such as features = ["serde"] would you prefer u32 or u64 i.e. 32

serde = ["ed25519-dalek/u32_backend", "serde_crate"]

64

serde = ["ed25519-dalek/u64_backend", "serde_crate"]

It is worth considering that a new feature like serde may be used a) in isolation or b) in conjunction with another feature like the wasm feature. This leads me to believe that a new feature like the serde feature should use u32_backend. Happy to be guided by you on this though.

Kind regards Tim

[clintfred]

We use the u32 backend for WASM as it is more performant.

I'm not sure we need to couple the serde support and the ed25519 backend at all. To use recrypt you need to select a backend, and you can also select serde support. If you define serde as

serde = ["serde_crate"]

For 64-bit backend + serde:

cargo build --features serde

For 32-bit backend + serde (which would be non-standard, but not incorrect)

cargo build --no-default-features --features serde,ed25519-dalek/u32_backend

For wasm + serde

cargo build --no-default-features --features serde,wasm

[tpmccallum]

Awesome! Thanks @clintfred I have updated the Cargo.toml in the related PR and also added this logic to the documentation in the same PR's README.md

[tpmccallum]

Hi @clintfred That approach made total sense to me. However, unfortunately that approach did not bode well with the compiler :( If we use the following build command as suggested

cargo build --no-default-features --features serde,ed25519-dalek/u32_backend

We receive the following error

... It has a required dependency with that name, but only optional dependencies can be used as features

Essentially the issue here is that any 3rd-party application which wants to use both serde and recrypt with the optional serde feature can not compile.

Interestingly, if we use the following syntax in the Cargo.toml file (of the 3rd-party application) instead

features = ["serde", "ed25519-dalek/u32_backend"]

We receive the following error from the compiler

error: feature names may not contain slashes: `ed25519-dalek/u32_backend`

As a result of the above, may I suggest that recrypt explicitly specifies two modes: 1) default = ["ed25519-dalek/u64_backend"] which already exists 2) u32 = ["ed25519-dalek/u32_backend"]

If the above modes are implemented then 3rd-party applications could use the following dependencies in their Cargo.toml.

Adding the optional Serde feature - 64-bit + serde

[dependencies]
recrypt = { version = "~0.11.0", features = ["serde"] } 

Adding the optional Serde feature - 32-bit + serde

[dependencies]
recrypt = { version = "~0.11.0", features = ["serde", "u32"], default-features = false } 

Adding the optional Serde feature - wasm + serde

[dependencies]
recrypt = { version = "~0.11.0", features = ["serde", "wasm"], default-features = false } 

To keep things neat and organised, I have gone ahead to create a new PR for this explicit u32 feature. I think this small change will be enough to provide all the flexibility we require.

Kind regards Tim

Just out of interest, once this change is implemented, we could also change the wasm feature from

wasm = ["ed25519-dalek/u32_backend", "clear_on_drop/no_cc", "getrandom/wasm-bindgen"]

to

wasm = ["clear_on_drop/no_cc", "getrandom/wasm-bindgen"]

This would allow Wasm to operate as both u32

recrypt = { version = "~0.11.0", features = ["u32", "wasm"], default-features = false } 

or u64

recrypt = { version = "~0.11.0", features = ["wasm"] } 

I know that the combination of u64 & wasm is not desirable, but this just demonstrates flexibility.

[clintfred]

I was thinking in this direction today as well -- exposing out those backend features separately as recrypt feature flags. I'll take a look at the PRs again tomorrow.

On Wed, Apr 22, 2020, 9:08 PM Timothy McCallum notifications@github.com wrote:

Hi @clintfred https://github.com/clintfred That approach made total sense to me. However, unfortunately that approach did not bode well with the compiler :( If we use the following build command as suggested

cargo build --no-default-features --features serde,ed25519-dalek/u32_backend

We receive the following error

... It has a required dependency with that name, but only optional dependencies can be used as features

Essentially the issue here is that any 3rd-party application which wants to use both serde and recrypt with the optional serde feature can not compile.

Interestingly, if we use the following syntax in the Cargo.toml file (of the 3rd-party application) instead

features = ["serde", "ed25519-dalek/u32_backend"]

We receive the following error from the compiler

error: feature names may not contain slashes: ed25519-dalek/u32_backend

As a result of the above, may I suggest that recrypt explicitly specifies two modes:

1. default = ["ed25519-dalek/u64_backend"] which already exists https://github.com/IronCoreLabs/recrypt-rs/blob/master/Cargo.toml#L52
2. u32 = ["ed25519-dalek/u32_backend"]

If the above modes are implemented then 3rd-party applications could use the following dependencies in their Cargo.toml.

Adding the optional Serde feature - 64-bit + serde

[dependencies] recrypt = { version = "~0.11.0", features = ["serde"] }

Adding the optional Serde feature - 32-bit + serde

[dependencies] recrypt = { version = "~0.11.0", features = ["serde", "u32"], default-features = false }

Adding the optional Serde feature - wasm + serde

[dependencies] recrypt = { version = "~0.11.0", features = ["serde", "wasm"], default-features = false }

To keep things neat and organised, I have gone ahead to create a new PR https://github.com/IronCoreLabs/recrypt-rs/pull/115 for this explicit u32 feature. I think this small change will be enough to provide all the flexibility we require.

Kind regards Tim

Just out of interest, once this change is implemented, we could also change the wasm feature from

wasm = ["ed25519-dalek/u32_backend", "clear_on_drop/no_cc", "getrandom/wasm-bindgen"]

to

wasm = ["clear_on_drop/no_cc", "getrandom/wasm-bindgen"]

This would allow Wasm to operate as both u32

recrypt = { version = "~0.11.0", features = ["u32", "wasm"], default-features = false }

or u64

recrypt = { version = "~0.11.0", features = ["wasm"] }

I know that the combination of u64 & wasm is not desirable, but this just demonstrates flexibility.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/IronCoreLabs/recrypt-rs/issues/112#issuecomment-618150218, or unsubscribe https://github.com/notifications/unsubscribe-auth/AESSWV4WEVEZOZMSWHPDKXLRN6WKFANCNFSM4MN26LTQ .