Skip to content

Enforce payable vs non-payable constructors#458

Merged
mbenke merged 3 commits into
argotorg:mainfrom
axic:payable-constructor
Jun 12, 2026
Merged

Enforce payable vs non-payable constructors#458
mbenke merged 3 commits into
argotorg:mainfrom
axic:payable-constructor

Conversation

@axic

@axic axic commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Constructors can now be marked payable. A non-payable constructor (the default) rejects any incoming value transfer at deploy time, reverting with the same NonPayableReceivedValue error (0xb5988ea3) used by the runtime method-level callvalue check. A payable constructor accepts value.

  • Add constrPayable to the surface and resolved Constructor AST
  • Parse payable constructor(...) (backtrack so payable function/fallback still parse)
  • Emit a NonPayable callvalue check in the generated deployer for non-payable constructors
  • Pretty-print the payable modifier
  • Let constructor tests assert an expected outcome (status/returndata); absent "output" still means "expect success"
  • Add payable_ctor / nonpayable_ctor dispatch tests and assert the existing PayableTest constructor accepts zero value

@axic axic marked this pull request as ready for review June 11, 2026 13:08
@axic axic force-pushed the payable-constructor branch 2 times, most recently from 14d4efd to 11de758 Compare June 12, 2026 07:39
@axic

axic commented Jun 12, 2026

Copy link
Copy Markdown
Contributor Author

Rebased after comptime.

@axic axic force-pushed the payable-constructor branch from 11de758 to f81d935 Compare June 12, 2026 12:01
Comment on lines +162 to +171
else if (expectedStatus == "success")
{
if (!status)
{
std::cerr << "Expected creation success but got failure" << std::endl;
resultRecorder.record(filename, "Expected constructor status success but got failure.", "failure", expectedStatus, gasUsed, gasUsedForDeposit);
hasTestFailure = true;
continue;
}
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what if expected status is not one of ["failure","success"] (e.g. typo)?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is mostly a code duplication of the non-constructor part below. I can add an error in both cases.

text "constructor"
ppr (Constructor ps bd payable) =
(if payable then text "payable" else empty)
<+> text "constructor"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This introduces an extra space if not payable

Suggested change
<+> text "constructor"
if payable then text "payable" <+> text "constructor" else text "constructor"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it better two have two tokens, than one text "payable constructor" ?

Comment on lines +164 to +165
(if payable then text "payable" else empty)
<+> text "constructor"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This introduces an extra space if not payable

Suggested change
(if payable then text "payable" else empty)
<+> text "constructor"
if payable then text "payable" <+> text "constructor" else text "constructor"

@axic

axic commented Jun 12, 2026

Copy link
Copy Markdown
Contributor Author

@mbenke pushed the fixes

claude and others added 3 commits June 12, 2026 16:29
Constructors can now be marked `payable`. A non-payable constructor
(the default) rejects any incoming value transfer at deploy time,
reverting with the same NonPayableReceivedValue error (0xb5988ea3) used
by the runtime method-level callvalue check. A payable constructor
accepts value.

- Add `constrPayable` to the surface and resolved Constructor AST
- Parse `payable constructor(...)` (backtrack so `payable function/fallback`
  still parse)
- Emit a NonPayable callvalue check in the generated deployer for
  non-payable constructors
- Pretty-print the `payable` modifier
- Let constructor tests assert an expected outcome (status/returndata);
  absent "output" still means "expect success"
- Add payable_ctor / nonpayable_ctor dispatch tests and assert the
  existing PayableTest constructor accepts zero value

Co-authored-by: Alex Beregszaszi <alex@rtfs.hu>
@axic axic force-pushed the payable-constructor branch from 1e483f1 to 352e79a Compare June 12, 2026 14:30
@mbenke mbenke merged commit 9ddf366 into argotorg:main Jun 12, 2026
4 checks passed
@axic axic deleted the payable-constructor branch June 12, 2026 14:44
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.

3 participants