[2/4] #[account(...)] macro generates the native account wrapper (instead of hardcoded Account type)

[greenhat]

Close #1157

This PR is stacked on #1113 and should be merged after it.

Unifies the #[account(...)] wrapper so a single user-declared type represents both the transaction's native (active) account and a foreign (FPI) account, replacing the auto-generated crate::bindings::Account struct.

• #[account(...)] wrapper methods now dispatch at runtime: a wrapper created with new(account_id) (renamed from from_account) routes calls through execute_foreign_procedure, while the wrapper passed to a #[note]/#[tx_script] entrypoint invokes the active account's native imports directly.
• BREAKING: the auto-generated bindings::Account struct is removed; entrypoint account parameters must reference an #[account(...)] type. This is enforced via the AccountWrapper marker trait rather than by type name, so a wrapper may itself be named Account, and non-wrapper types fail with a guiding trait-bound diagnostic.
• Dependency methods that would shadow ActiveAccount built-ins (e.g. get_id) are rejected at macro expansion to prevent silent method-resolution changes.

[greenhat]

@bitwalker Rebased and ready.

[bitwalker]

Looks good! I have a few notes/questions, but nothing major I think

[bitwalker]

Is this actually true? I believe Rust requires disambiguating with turbofish syntax in such cases (since it cannot know which trait method you are actually intending to call when both are in scope).

You do mention inherent method though, but IIRC the plan was to have component methods defined in the trait implementation, so it's a bit unclear to me why we're generating inherent methods at all vs trait implementations. Account methods like get_id I'd expect to be provided by implementing some kind of Account trait, rather than as inherent methods of the account struct itself. That would also avoid the issue with method resolution.

None of that prevents someone from adding something like get_id or whatever to their account/component struct, but if all of our stuff is trait-implemented, then they'd still be required to disambiguate with turbofish syntax when the conflicting trait is in scope (which would basically always be the case I think).

[greenhat]

You do mention inherent method though, but IIRC the plan was to have component methods defined in the trait implementation, so it's a bit unclear to me why we're generating inherent methods at all vs trait implementations. Account methods like get_id I'd expect to be provided by implementing some kind of Account trait, rather than as inherent methods of the account struct itself. That would also avoid the issue with method resolution.

The inherent method is defined to route inter-component vs FPI calls on the struct under the #[account(...)] macro on the callee side. It depends on the optional account id (for FPI) field defined in this struct. I don't think we planned to implement it as a trait. Here is an example of such a wrapper for the BasicWallet::receive_asset:

    pub fn receive_asset(&self, asset: ::miden::Asset) -> () {
        match self.foreign_account_id {
            ::core::option::Option::Some(__miden_foreign_account_id) => {
                __miden_foreign_account_wallet::miden::basic_wallet::miden_basic_wallet::fpi_receive_asset(
                    __miden_foreign_account_id.prefix,
                    __miden_foreign_account_id.suffix,
                    ::miden::Word::new([
                        {
                            const VALUE: u64 = 14498277730569837599u64 as u64;
                            ::miden_stdlib_sys::Felt::new(VALUE).unwrap()
                        },
                        {
                            const VALUE: u64 = 11350922803923988613u64 as u64;
                            ::miden_stdlib_sys::Felt::new(VALUE).unwrap()
                        },
                        {
                            const VALUE: u64 = 3366787023335181781u64 as u64;
                            ::miden_stdlib_sys::Felt::new(VALUE).unwrap()
                        },
                        {
                            const VALUE: u64 = 15705799660361837834u64 as u64;
                            ::miden_stdlib_sys::Felt::new(VALUE).unwrap()
                        },
                    ]),
                    asset,
                )
            }
            ::core::option::Option::None => {
                __miden_foreign_account_wallet::miden::basic_wallet::miden_basic_wallet::receive_asset(
                    asset,
                )
            }
        }
    }

[bitwalker]

Maybe we should just never emit any of these, and just leave it up to the user - that simplifies things here, and only requires adding one line to account component declarations that need any of these trait impls (which we're seemingly anticipating that they'd be adding anyway).

[greenhat]

We need Default for AccountWrapper::native() to work since AccountWrapper: Default and native() is just Self::default(). I removed the rest in ac7af62

[bitwalker]

I would make this method required, rather than providing a default no-op implementation, since presumably we'll have to emit impl ActiveAccount for all #[account(...)] structs anyway.

[greenhat]

There's exactly one macro producing dual native/foreign types (#[account]), and it always emits the override (panic! if the foreign account is set). If we make it required, it'd move the no-op implementation into boilerplate, and we'd have to impl ActiveAccount for T { fn __assert_active_account(&self) {} } on each impl.

[bitwalker]

Doesn't this also need ActiveAccount as a supertrait as well? Otherwise you could define an impl of AccountWrapper for something like i32, which we probably don't want to treat as valid.

[greenhat]

Good catch! Thank you! Done in 5d297a2

[bitwalker]

I'm a bit confused about the mixture of native vs active terminology - the latter makes sense to me, the former does not (an account being "native" really conveys nothing meaningful to me at all - "active"/"current" is way more intuitive IMO).

[greenhat]

Renamed to active() in f905a16

[greenhat]

Thank you! I addressed all the review notes. Please do another round.