Derive Default for MmrPeaks

[plafer]

Useful to create a dummy MmrPeaks value in tests

[hackaugusto]

Humm, I had intentionally not derived Default for this struct. Since I initially thought it wasn't a useful value. However, after our discussions about the Mmr lagging for a block, the genesis block must have a commitment to an the empty peaks.

I think we should also add some tests with this value, to make sure that are no off-by-one bugs lurking in the codebase.

[plafer]

@hackaugusto I changed it so that Default is only derived in tests. It would allow us to derive(Default) on structs that contain an MmrPeaks in tests when we don't care about the MmrPeaks value for a given test

[hackaugusto]

IMO for testing we should use the mock utilities: https://github.com/0xPolygonMiden/miden-base/tree/main/mock/src/mock

These functions should take care of removing the boilerplate for data generation, while generating valid values.

[bobbinth]

I would also prefer to avoid implementing Default for testing purposes only if there is a different way.

Also, I believe using #[cfg_attr(test, derive(Default))] would make the implementation local to this crate (i.e., when the crate is imported, it won't have Default implemented.

If however, doing #[cfg_attr(test)] works, I'd probably prefer adding an explicit dummy() constructor (or something like that) with this attribute, and then this constructor could be used in tests.

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

[plafer]

If however, doing #[cfg_attr(test)] works, I'd probably prefer adding an explicit dummy() constructor

Ah, you're right, items annotated with [cfg(test)] are not exported, even when the crate is a dev-dependency.

IMO for testing we should use the mock utilities

Agreed