Support exporting trait default fns using a fn without a block

[leighmcculloch]

What

Add support for exporting trait default functions by marking them for export by using a fn without an implementation block.

Example

Contract developer implements trait that has some default behaviour:

#![no_std]
use soroban_sdk::{contract, contractimpl, Env};
use library::Pause;

#[contract]
pub struct Contract;

#[contractimpl]
impl Pause for Contract {
    fn pause(env: &Env, paused: bool);
    fn paused(env: &Env) -> bool;
}

Library developer defines trait with some default behaviour:

#![no_std]
use soroban_sdk::{contracttrait, symbol_short, Env};

pub trait Pause {
    fn pause(env: &Env, paused: bool) {
        env.storage().persistent().set(&symbol_short!("paused"), &paused)
    }

    fn paused(env: &Env) -> bool {
        env.storage().persistent().get(&symbol_short!("paused")).unwrap_or(false)
    }
}

Why

This is an alternative solution to #1507 to the problem presented by #1451.

When defining traits that have default functions, and implementing them for a contract, the default functions do not get exported and there's no way for a developer to export them without reimplementing them. The default functions do get exported when overridden by the contract. That difference in behaviour is surprising, and surprising behaviour can lead to bugs.

This solution doesn't remove the surprise as well as #1507 does, because the developer still needs to learn to do something new. But it does provide a way for the developer to signal they wish to export the function without having to provide an implementation that somehow calls the original.

This solution behaves the same way that the inherent crate does. How inherent supports default fns is not well documented, but you can see it here and the solution in this PR is the same experience: dtolnay/inherent#9.

Close #1451

Comparison to #1507

This solution is simpler than #1507, and has an interesting property that the code is explicit about what default functions get exported from the contract. One downside of #1507, and trait default functions, is that they are invisible to a reader of the contract when looking at the contract code itself.

While #1507 is a breaking change that breaks any contract using traits with contractimpl blocks today. This change is not a breaking change for any existing contract and is additive only.

See the diffs below for how this solution compares to #1507:

Contract developer implements trait that has some default behaviour:

 #![no_std]
 use soroban_sdk::{contract, contractimpl, Env};
 use library::Pause;
 
 #[contract]
 pub struct Contract;
 
 #[contractimpl]
 impl Pause for Contract {
+    fn pause(env: &Env, paused: bool);
+    fn paused(env: &Env) -> bool;
 }

Library developer defines trait with some default behaviour:

 #![no_std]
 use soroban_sdk::{contracttrait, symbol_short, Env};
 
-#[contracttrait]
 pub trait Pause {
     fn pause(env: &Env, paused: bool) {
         env.storage().persistent().set(&symbol_short!("paused"), &paused)
     }
 
     fn paused(env: &Env) -> bool {
         env.storage().persistent().get(&symbol_short!("paused")).unwrap_or(false)
     }
 }

Why not

This change introduces a pattern that isn't standard Rust. While it is a pattern used in the existing inherent crate, meaning it is an existing solution that already exists in the Rust ecosystem, it is not a very common pattern.

[leighmcculloch]

TODO:

• Make it error if the functions defined as defaults in the final impl do not exactly match the trait.

[leighmcculloch]

TODO:

• Find a way for the unused params in block-less fns to be ignored by the linters so warnings are not output for them.
• Investigate breakage to OZ contracts

[willemneal]

Some initial thoughts: firstly I do like the clarity of it, however, it does feel strange when compared to the way you expect default methods to work. Secondly, one feature of the original method was that you could copy the doc comments into the contract's method.

[leighmcculloch]

firstly I do like the clarity of it

One thing I don't like the clarity of it, is it now becomes really easy to look like a full trait interface is exposed, but it's possible, through human error or intent, for say one function not to be exposed because it has just been missed. And the only way to address that type of mistake or catch it is to again use the more complex mechanisms in #1507.

Not being able to surface the doc comments is a really good point. 👍🏻

So while I appreciated trying this approach out and some of its benefits, I don't think it holistically creates the right experience.