Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix bug in contractimpl code generation for impl traits using associated types #1401

Merged
merged 8 commits into from
Dec 6, 2024

Conversation

leighmcculloch
Copy link
Member

What

Allow associated types in impl of traits, specifically the CustomAccountInterface.

Why

The use of associated types was allowed until v22 of the SDK, where an assumption in the code generation contractimpl macro is causing ambiguous code to be generated when associated types are in use.

Close #1400

@leighmcculloch
Copy link
Member Author

I wanted to do just as the error message said and replace the Self::Signature with <Contract as CustomAccountInterface>::Signature. We have all the information to do that, and that's what I did in this commit:

However, that then broke the contract spec generation, the map_type function specifically, which can't handle those qself types.

But then digging further, it looks like Self::Signature never actually worked properly because map_type doesn't know how to map it to the underlying type.

So, in the interest of trying to do something I thought would be simple, I've gone with the approach that when the SDK macro first parses the impl block, if the block is implementing a trait, it flattens any associated types in the functions. At the moment in this PR it does it only for function inputs, but after this merges I'd like to follow up with doing the same thing with function return values.

@leighmcculloch
Copy link
Member Author

I also had to increase the version requirement on the syn package. The sdk said it required any 2.0 version, but in practice we were using 2.0.77 in the repo. In this PR I started using some functions that were introduced somewhere along the way. The fuzz example has its own lock file and it was using 2.0.37, which it turns out doesn't have those functions. Minimum-version-selection, if Cargo ever adopts it, would prevent situations like this.

@anupsdf anupsdf requested a review from graydon December 6, 2024 17:40
@sisuresh sisuresh added this pull request to the merge queue Dec 6, 2024
Merged via the queue into main with commit 5ad52f7 Dec 6, 2024
17 checks passed
@sisuresh sisuresh deleted the i1400 branch December 6, 2024 19:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Self references in CustomAccountInterface impls are broken by SDK v22
2 participants