stacks-network / stacks-core

The Stacks blockchain implementation
https://docs.stacks.co
GNU General Public License v3.0
3.01k stars 667 forks source link

Traits with Duplicate Names in Function Signatures are Allowed #3214

Closed sskeirik closed 1 year ago

sskeirik commented 2 years ago

Describe the bug On master branch, when a contract contains a trait definition like the following:

(define-trait double-method (
  (foo (uint) (response uint uint))
  (foo (bool) (response bool bool))
))

Then only the last copy of foo is kept as part of the processed trait's definition. This means that we can then successfully launch a contract like the following:

(impl-trait 'SP8CW062DS1XAZJJXWKSM9EMMDD51BRVFMY8MBX6.double-trait.double-method)
(define-read-only (foo (x bool)) (ok x))

Steps To Reproduce If the first contract is stored in a file double-trait.clar and the second contract is stored in a file impl-double-trait.clar, then the CLI invocation to test would be:

clarity-cli initialize initial-allocations.json vm-state.db
clarity-cli launch SP8CW062DS1XAZJJXWKSM9EMMDD51BRVFMY8MBX6.double-trait double-trait.clar vm-state.db
clarity-cli launch SP8CW062DS1XAZJJXWKSM9EMMDD51BRVFMY8MBX6.impl-double-trait impl-double-trait.clar vm-state.db

Expected behavior I expect that the type checker will reject contracts with traits that contain two function signatures with the same name, even if they are identical otherwise.

Environment (please complete the following information):

Additional context N/A

jcnelson commented 2 years ago

Thanks for your report! We're looking into it. A proper fix would be to address this in the 2.1 upgrade, but in all likelihood the tooling could also warn against this.

jcnelson commented 1 year ago

Fixed by #3251