Raise an error if changing data on an account without mut

[Henry-E]

#[account(mut)] I recently forget to put mut above an associated account whose data was being changed. This didn't raise an error, it just failed silently. Ideally this would raise an error.

Other functions, like in the SPL token program, raise errors when trying to change account data without the mut flag

[armaniferrante]

How do other programs do that?

Anchor could inject a check for fields with mut on them and return an error code if it's not mut.

[Henry-E]

I'm not sure how it's done in other programs exactly but usually I get an error when trying to do CPI with non mutable accounts. Maybe it's because they assume that there will be account changes, whereas in anchor an associated account might not be changed in a given transaction. Maybe if, like you suggested, the compiler detected a mutable reference to an account but didn't also detect a mutable account attribute that it should raise an error.

All I can say is it was a confusing error to debug because it was silently failing to update the value on the account permanently.

This is the usual error that pops up

[snawaz]

@armaniferrante

Anchor could inject a check for fields with mut on them and return an error code if it's not mut.

Or it can implement AccountsExit ensuring exit() writes data to AccountInfo for all fields, irrespective of whether mut is present or not (and then Solana will catch the error if is_writable == false for an account). That will make the exit step very non-performant, but that seems to be the only simple way because ctx.accounts is a mutable reference and users can write any accounts. Unless individual members of a mutable struct can be made read-only and I don't see any easy way. Or DerefMut needs to assert is_writable == true, else panic!().

[snawaz]

Alright. I think we have a better solution than what I suggested in the previous comment. Not only we can optimally do it, we can also get compilation error if trying to mutate an account which is not marked as mut.

The problem I see is with Account<> type which implements DerefMut which means it allows mutation irrespective of the absence/presence of mut attribute. What we need is.. another type here.. say Immutable<> which is just like Account<> except that it implements only Deref, not DerefMut. Then either we allow users to use this type directly.. OR when can change Account<> field to Immutable<> during code generation if mut is not present on the field.

Please let me know if this sounds good?

cc: @acheroncrypto

[snawaz]

OK. I implemented this in this PR: https://github.com/coral-xyz/anchor/pull/2513