feat(types): Enable failable conversions to/from sdk and core types

[jkrvivian]

Description of change

Merging upstream changes: MystenLabs/sui#19414
This one is needed for the next fix: #4687

Avoid panicking in sdk/core conversion by replacing From to TryFrom and introduce the new error type SdkTypeConversionError.

Links to any relevant issues

Close #4776

Type of change

• Enhancement

Change checklist

• I have followed the contribution guidelines for this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• I have added tests that prove my fix is effective or that my feature works
• I have checked that new and existing unit tests pass locally with my changes

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
apps-backend	❌ Failed (Inspect)			Jan 20, 2025 7:24am
apps-ui-kit	❌ Failed (Inspect)			Jan 20, 2025 7:24am
rebased-explorer	❌ Failed (Inspect)			Jan 20, 2025 7:24am
wallet-dashboard	❌ Failed (Inspect)			Jan 20, 2025 7:24am

[DaughterOfMars]

So, I don't think we should do this. These types should be infallibly convertible. The only things that aren't right now are the Identifier and signatures and those are only because there are no functions defined to do it because this behavior isn't part of their public API. However, those conversions are still infallible as the types are represented the same. This introduces a lot of unnecessary complexity IMO for no gain.

However, please tell me if there is a reason I am missing!

[jkrvivian]

So, I don't think we should do this. These types should be infallibly convertible. The only things that aren't right now are the Identifier and signatures and those are only because there are no functions defined to do it because this behavior isn't part of their public API. However, those conversions are still infallible as the types are represented the same. This introduces a lot of unnecessary complexity IMO for no gain.

However, please tell me if there is a reason I am missing!

I totally understand your perspective, but here’s why we believe this change is still worthwhile:

1. Technical Debt and Alignment with Sui

   This change helps address technical debt, reducing future maintenance challenges. It also aligns with Sui’s updates. It’s possible Sui encountered bugs that led to this fix, even if they didn’t explicitly state the reasons in the PR description. Following their changes, particularly in error handling and bug fixes, could help prevent potential issues in the future.

2. Error Handling and Future-Proofing

   Adopting try_from enhances extensibility and prepares the codebase for unforeseen requirements, saving effort in the long term.

[muXxer]

It is about users being able to feed bad data. That's the reason sui did this IMO.