Skip to content

Finalize single-expression light handlers #2381

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.

Sign up for GitHub

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

Open
TUPM96 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Finalize single-expression light handlers #2381

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 25 additions & 2 deletions sdk-libs/macros/src/light_pdas/program/parsing.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -617,15 +617,38 @@ fn is_delegation_body(block: &syn::Block, ctx_name: &str) -> bool {
syn::Stmt::Expr(expr, _) => {
// Check if it's a function call that takes ctx as an argument
match expr {
syn::Expr::Call(call) => call_has_ctx_arg(&call.args, ctx_name),
syn::Expr::MethodCall(call) => call_has_ctx_arg(&call.args, ctx_name),
syn::Expr::Call(call) => call_moves_ctx_arg(&call.args, ctx_name),
syn::Expr::MethodCall(call) => call_moves_ctx_arg(&call.args, ctx_name),
_ => false,
}
}
_ => false,
}
}

/// Check if any argument consumes the context param.
///
/// Passing `&ctx` or `&mut ctx` lets the wrapper continue using `ctx` for
/// finalization after the handler returns, so those are intentionally excluded.
fn call_moves_ctx_arg(
args: &syn::punctuated::Punctuated<syn::Expr, syn::token::Comma>,
ctx_name: &str,
) -> bool {
for arg in args {
match arg {
syn::Expr::Path(path) if path.path.is_ident(ctx_name) => return true,
syn::Expr::MethodCall(method_call)
if method_call.method == "into"
&& matches!(&*method_call.receiver, syn::Expr::Path(path) if path.path.is_ident(ctx_name)) =>
{
return true;
}
_ => {}
}
}
false
}
Comment on lines +621 to +650
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 25, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -n -C3 'is_delegation_body|Expr::MethodCall|call_moves_ctx_arg|call\\.receiver' sdk-libs/macros/src/light_pdas/program/parsing.rs
rg -n -C2 'test_wrap_.*finalize|instructions::handler\\(&mut ctx\\)|instructions::handler\\(ctx\\)' sdk-libs/macros/src/light_pdas_tests/parsing_tests.rs

Repository: Lightprotocol/light-protocol

Length of output: 3518

Fix delegation detection to account for moved ctx used as a method-call receiver

In sdk-libs/macros/src/light_pdas/program/parsing.rs (~lines 611-623), is_delegation_body checks syn::Expr::MethodCall(call) for delegation using only call.args via call_moves_ctx_arg, ignoring call.receiver. If a handler consumes ctx as the receiver (e.g., ctx.delegate(...)), delegation can be misclassified and the wrapper may incorrectly skip/emit the finalize path. sdk-libs/macros/src/light_pdas_tests/parsing_tests.rs only covers instructions::handler(ctx) / instructions::handler(&mut ctx) (arg-style Expr::Call), not receiver consumption.

Suggested fix 
 fn is_delegation_body(block: &syn::Block, ctx_name: &str) -> bool {
@@
         syn::Stmt::Expr(expr, _) => {
             // Check if it's a function call that takes ctx as an argument
             match expr {
                 syn::Expr::Call(call) => call_moves_ctx_arg(&call.args, ctx_name),
-                syn::Expr::MethodCall(call) => call_moves_ctx_arg(&call.args, ctx_name),
+                syn::Expr::MethodCall(call) => {
+                    expr_moves_ctx(&call.receiver, ctx_name)
+                        || call_moves_ctx_arg(&call.args, ctx_name)
+                }
                 _ => false,
             }
         }
@@
 }
 
+fn expr_moves_ctx(expr: &syn::Expr, ctx_name: &str) -> bool {
+    match expr {
+        syn::Expr::Path(path) if path.path.is_ident(ctx_name) => true,
+        syn::Expr::MethodCall(method_call)
+            if method_call.method == "into"
+                && matches!(&*method_call.receiver, syn::Expr::Path(path) if path.path.is_ident(ctx_name)) =>
+        {
+            true
+        }
+        _ => false,
+    }
+}
+
 fn call_moves_ctx_arg(
     args: &syn::punctuated::Punctuated<syn::Expr, syn::token::Comma>,
     ctx_name: &str,
 ) -> bool {
     for arg in args {
-        match arg {
-            syn::Expr::Path(path) if path.path.is_ident(ctx_name) => return true,
-            syn::Expr::MethodCall(method_call)
-                if method_call.method == "into"
-                    && matches!(&*method_call.receiver, syn::Expr::Path(path) if path.path.is_ident(ctx_name)) =>
-            {
-                return true;
-            }
-            _ => {}
+        if expr_moves_ctx(arg, ctx_name) {
+            return true;
         }
     }
     false
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
syn::Expr::MethodCall(call) => call_moves_ctx_arg(&call.args, ctx_name),
_ => false,
}
}
_ => false,
}
}
/// Check if any argument consumes the context param.
///
/// Passing `&ctx` or `&mut ctx` lets the wrapper continue using `ctx` for
/// finalization after the handler returns, so those are intentionally excluded.
fn call_moves_ctx_arg(
args: &syn::punctuated::Punctuated<syn::Expr, syn::token::Comma>,
ctx_name: &str,
) -> bool {
for arg in args {
match arg {
syn::Expr::Path(path) if path.path.is_ident(ctx_name) => return true,
syn::Expr::MethodCall(method_call)
if method_call.method == "into"
&& matches!(&*method_call.receiver, syn::Expr::Path(path) if path.path.is_ident(ctx_name)) =>
{
return true;
}
_ => {}
}
}
false
}
syn::Expr::MethodCall(call) => {
expr_moves_ctx(&call.receiver, ctx_name)
|| call_moves_ctx_arg(&call.args, ctx_name)
}
_ => false,
}
}
_ => false,
}
}
fn expr_moves_ctx(expr: &syn::Expr, ctx_name: &str) -> bool {
match expr {
syn::Expr::Path(path) if path.path.is_ident(ctx_name) => true,
syn::Expr::MethodCall(method_call)
if method_call.method == "into"
&& matches!(&*method_call.receiver, syn::Expr::Path(path) if path.path.is_ident(ctx_name)) =>
{
true
}
_ => false,
}
}
/// Check if any argument consumes the context param.
///
/// Passing `&ctx` or `&mut ctx` lets the wrapper continue using `ctx` for
/// finalization after the handler returns, so those are intentionally excluded.
fn call_moves_ctx_arg(
args: &syn::punctuated::Punctuated<syn::Expr, syn::token::Comma>,
ctx_name: &str,
) -> bool {
for arg in args {
if expr_moves_ctx(arg, ctx_name) {
return true;
}
}
false
}
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@sdk-libs/macros/src/light_pdas/program/parsing.rs` around lines 621 - 650,
is_delegation_body currently only inspects MethodCall.args via
call_moves_ctx_arg and misses cases where the context is moved as the
MethodCall.receiver (e.g., ctx.delegate(...)); update the logic to also check
the MethodCall.receiver for moved ctx. Specifically, modify the MethodCall
branch in is_delegation_body to test call.receiver for being an identifier equal
to ctx_name or being a MethodCall "into" whose receiver is that identifier
(analogous to the existing checks in call_moves_ctx_arg), or refactor
call_moves_ctx_arg to accept and check a receiver Expr as well and reuse the
same moved-ctx detection for receiver and args; ensure you reference
is_delegation_body, call_moves_ctx_arg, syn::Expr::MethodCall and call.receiver
when making the change.


/// Check if any argument in the call is the context param (moving the context).
/// Detects: ctx, &ctx, &mut ctx, ctx.clone(), ctx.into(), etc.
/// `ctx_name` is the context parameter name to look for (e.g., "ctx", "context").
Expand Down
37 changes: 36 additions & 1 deletion sdk-libs/macros/src/light_pdas_tests/parsing_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@
//!
//! Extracted from `light_pdas/program/parsing.rs`.

use quote::quote;
use syn::punctuated::Punctuated;

use crate::light_pdas::program::parsing::{
call_has_ctx_arg, extract_context_and_params, ExtractResult,
call_has_ctx_arg, extract_context_and_params, wrap_function_with_light, ExtractResult,
};

fn parse_args(code: &str) -> Punctuated<syn::Expr, syn::token::Comma> {
Expand Down Expand Up @@ -216,3 +217,37 @@ fn test_extract_context_and_params_multiple_args_detected() {
_ => panic!("Expected ExtractResult::MultipleParams"),
}
}

#[test]
fn test_wrap_single_expression_mut_context_runs_finalize() {
let fn_item: syn::ItemFn = syn::parse_quote! {
pub fn handler(ctx: Context<MyAccounts>, params: Params) -> Result<()> {
instructions::handler(&mut ctx)
}
};
let params_ident: syn::Ident = syn::parse_quote!(params);
let ctx_ident: syn::Ident = syn::parse_quote!(ctx);

let wrapped = wrap_function_with_light(&fn_item, &params_ident, &ctx_ident);
let wrapped_tokens = quote!(#wrapped).to_string();

assert!(wrapped_tokens.contains("__user_result"));
assert!(wrapped_tokens.contains("light_finalize"));
}

#[test]
fn test_wrap_moved_context_delegation_skips_finalize() {
let fn_item: syn::ItemFn = syn::parse_quote! {
pub fn handler(ctx: Context<MyAccounts>, params: Params) -> Result<()> {
instructions::handler(ctx)
}
};
let params_ident: syn::Ident = syn::parse_quote!(params);
let ctx_ident: syn::Ident = syn::parse_quote!(ctx);

let wrapped = wrap_function_with_light(&fn_item, &params_ident, &ctx_ident);
let wrapped_tokens = quote!(#wrapped).to_string();

assert!(!wrapped_tokens.contains("__user_result"));
assert!(!wrapped_tokens.contains("light_finalize"));
}