Remove clvm-traits in favor of adding implementations back to chia_rs

[Rigidity]

Having clvmr depend on clvm-traits instead of the other way around has been complicating things, and requiring that we use patch.crates-io in the chia_rs monorepo to prevent mismatching dependency errors. We don't actually lose any functionality by moving the trait implementations to clvm-traits, and it can simplify testing (since we no longer would need TestAllocator).

clvmr itself doesn't care about clvm-traits existing.

Note that this is a breaking change and should be released in a semver-incompatible version with the last to prevent conflicts.

[arvidn]

I would expect it to be easier to add the new (counterpart) implementation in chia_rs. I imagine you'll make some wrapper type for Allocator that implements ToClvm and FromClvm, right?

[Rigidity]

I would expect it to be easier to add the new (counterpart) implementation in chia_rs. I imagine you'll make some wrapper type for Allocator that implements ToClvm and FromClvm, right?

It should be as easy as just copying the code that was removed here into clvm-traits and making it depend on clvmr again. The original circular dependency issue was caused by chia-bls depending on clvm-traits, so this would no longer be an issue. I don't think a wrapper type will be needed, and all usage of the trait will remain the same.

[coveralls-official[bot]]

Pull Request Test Coverage Report for Build 7463233530

• 0 of 0 changed or added relevant lines in 0 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage decreased (-0.05%) to 93.635%

---

Totals
Change from base Build 7462775432:	-0.05%
Covered Lines:	5237
Relevant Lines:	5593

---

💛 - Coveralls

[arvidn]

what corresponding change would be needed in chia_rs? how do you avoid the original problem that prompted us to move clvm-traits here in the first place? You want to make clvm-traits depend on both clvmr and chia-bls? It seems to me that doing that would cause some inconveniences in the future, when we have a new crate that wants to implement the clvm traits, and it must pull in clvmr and chia-bls because of it. Those issues probably are some time out (if we ever get there), so it might be reasonable to trade off inconveniences we have today for potential future inconveniences.

[arvidn]

to be honest; I also don't quite understand why we need patch.crates-io either. I thought that was just temporary while transitioning to clvm-traits living in this repo

[Rigidity]

to be honest; I also don't quite understand why we need patch.crates-io either. I thought that was just temporary while transitioning to clvm-traits living in this repo

It's because clvmr uses clvm-traits in its public API, and while developing chia_rs the crates.io version is considered an unrelated crate to the local clvm-traits. So, any mixing of local clvm-traits types and the counterpart in clvmr causes type errors. patch.crates-io is a way to work around this by telling Cargo to use the local clvm-traits for both rather than fetching iot from crates.io. However this wouldn't work well if there are breaking changes in the local version, and it adds extra baggage to our manifest.

[arvidn]

ok, right. the patching is necessary when making changes to clvm-traits, since there's an external crate that depends on it.