Avm should reject default methods

[aionick]

Some Java interfaces define default methods that our userlib implementations consequently inherit. For example, Map defines a default method putIfAbsent(). The following contract makes a call into this method:

public class DefaultMethodContract {
    private static AionMap<Integer, Integer> map = new AionMap<>();

    public static byte[] main() {
        return ABIDecoder.decodeAndRunWithClass(DefaultMethodContract.class, BlockchainRuntime.getData());
    }

    public static int invokeDefaultMapMethod() {
        return map.putIfAbsent(0, 10);
    }
}

This contract can be successfully deployed. The method cannot be successfully called, so there is no risk of invoking real Java code here, but this failure should happen upon deployment. This short test will pass:

@Test
    public void testDefaultMethod() {
        KernelInterface kernel = new KernelInterfaceImpl();
        VirtualMachine avm = CommonAvmFactory.buildAvmInstance(kernel);

        // deploy contract
        byte[] testJar = JarBuilder.buildJarForMainAndClasses(DefaultMethodContract.class, AionMap.class);

        Transaction createTransaction = Transaction.create(from, kernel.getNonce(from).longValue(), BigInteger.ZERO, new CodeAndArguments(testJar, new byte[0]).encodeToBytes(), energyLimit, energyPrice);
        TransactionContext createContext = new TransactionContextImpl(createTransaction, block);
        TransactionResult createResult = avm.run(new TransactionContext[] {createContext})[0].get();
        assertTrue(createResult.getResultCode().isSuccess());
    }

[aionick]

Users can also write dApps that define subclasses of, say Map, and inherit the default methods as well. We also need to figure out how we want to handle this case.

So far, such a contract can be deployed. However, it will generate a NoSuchMethodError when the default method is invoked.

[aionick]

I see 3 ways of approaching this problem, since we cannot detect a default method directly in the bytecode:

1. We provide implementation of these default methods. The major downside to this is that we end up having to create and whitelist a whole bunch of classes that only exist to make the compiler happy (like Spliterator) so that we can match the method signatures exactly, and these classes have no real purpose beyond this in a lot of cases.

2. We maintain a list of default methods to forbid. The major downsides to this is that this list will change with new JDK releases and if we add any other interfaces to our whitelist. Also, the simple implementation of this is too far-reaching, it would actually amount to forbidding a specific method name (or at best, a specific method signature), which is not really desirable. But a legitimate implementation is quite complicated and would involve having to determine in the rejection visitor whether the method signature matches up with a default method in an interface that the class implements. Currently there is no easy way to get this kind of ability into the visitor, but it is of course do-able.

3. We leave everything as is. There is no risk in doing this. When the method is invoked a NoSuchMethodError will be thrown. But this is arguably cryptic and misleading: it exposes the transformed method name, which may be a bit puzzling, and it throws a no such method error on something that supposedly has a default implementation. In our context this isn't too bizarre, but it does feel unnatural.

[jeff-aion]

We suspect that the remaining aspects of this problem are just related to communicating Shadow JCL scope to the filtering/verification tool we will provide to developers. This way, if they try to rely on a default method, the tool can tell them that this is a difference between the interfaces described in the standard JCL and the Shadow JCL.

We should look into eagerly checking method bindings at deployment time, since the receiver type should be known to ASM at the point where the invokevirtual occurs. In any case, that is for a different item as it has a much broader scope.