Skip to content

Conversation

divarvel
Copy link
Contributor

@divarvel divarvel commented Jul 21, 2025

macros resolve scope parameters and regular parameters through the ToAnyParams trait, losing the ability to discriminate between the two in a static way. This leads to panics instead of compilation errors.

Note: this PR fixes the issue of mistakenly providing a term-like value (most likely a string) where a public key is expected. It also removes the provided impl ToAnyParam for PublicKey. Since this implementation never makes sense with the other changes, it’s fine to remove it as a bugfix.
The code still uses ToAnyParam for datalog terms, which allows to provide a public key value when implementing it.
Completely removing this possibility requires removing the ToAnyParam conversion trait altogether, which requires careful thought. This is what #314 is about.

That being said, this PR already removes the possibility of honest mistakes when using macros, so it’s valuable in itself.

Fixes #301

ToDo

  • store separately parameters and scope parameters in Item
  • only use ToAnyParam for datalog terms
  • require PublicKey directly for scope params
  • remove impl ToAnyParam for PublicKey
  • try using Into<Term> instead of T: ToAnyParam
  • try using Into<(PublicKey, Vec<PublicKey>)> instead of PublicKey

@divarvel divarvel force-pushed the push-twzwuoutszpo branch 3 times, most recently from fafb13e to dedc5d7 Compare August 7, 2025 13:12
@divarvel divarvel changed the title WIP: fix parameter confusion in macros fix parameter confusion in macros Sep 16, 2025
@divarvel divarvel force-pushed the push-twzwuoutszpo branch 2 times, most recently from 300a19d to c841ee1 Compare September 16, 2025 09:38
@divarvel divarvel marked this pull request as ready for review September 16, 2025 09:39
macros resolve scope parameters and regular parameters through the `ToAnyParams` trait, losing the ability to discriminate between the two in a static way. This leads to panics instead of compilation errors.
@divarvel divarvel merged commit 3ad5966 into main Sep 17, 2025
4 checks passed
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.

Panic with macro rule using trusting

2 participants