Spike/add msa pallet #2
Conversation
…d added stuff to the jot-examples
tedward111
force-pushed
the
spike/add_msa_pallet
branch
from
046c2a2 to
a8d0b96
Compare
pfrank13
left a comment
pfrank13
left a comment
There was a problem hiding this comment.
Not a bad first pass. Lot's of ways to improve it because if we can get the concepts in place that's great. At the end of the day the Mapper concept is like our old ScaleReader and I think those can potentially be done purely by reflection on the instance class (eventually this is non trivial) but understanding that T:Accountid and field1 are key to that.
|
|
||
| public static void main(String[] args) throws Exception { | ||
| Wallet wallet = Wallet.fromMnemonic(Config.MNEMONIC_PHRASE); | ||
| Wallet alice = Wallet.fromSr25519Seed(HexUtil.hexToBytes("0xe5be9a5092b81bca64be81d212e7f2f9eba183bb7a90954f7b76361f6edb5c0a")); |
There was a problem hiding this comment.
Why was this changed from the Mnemonic?
| public final Kind kind; | ||
| public final int moduleIndex; | ||
| public final int errorCode; | ||
| public final String name; |
There was a problem hiding this comment.
Never do this in Java for various reasons provide getters
| @@ -0,0 +1,3 @@ | |||
| package com.method5.jot.extrinsic.call; | |||
|
|
|||
| public class KeyAlreadyRegisteredError implements ExtrinsicError{} | |||
There was a problem hiding this comment.
So rather than make a class per Error just use an enum. Something with a surrogate key called something like palletValue and compare against that in order to figure out what enu, value to return. See our fromXXX static methods on lot's of enums. You can still use a marker interface like ExtrinsicError as well but we'd want to "namespace them" by pallet to not what a gigantic enum for everything. Could even make a MsaExtrinsiveError extend the ExtrinsicError marker interface.
| return writer.toByteArray(); | ||
| } | ||
|
|
||
| public static class MsaCreated implements EventClass<MsaCreated>{ |
There was a problem hiding this comment.
This EventClass is probably better modeled as MsaCreatedMapper implements Mapper<MsaCreated> (this is effectively the "ScaleReader" concept but since we have metadata we don't need to do as much) and make a mapper interface (with no static) that has a "map" or "create" method that only works. This will also would lift this static inner class out
|
|
||
| import java.util.Map; | ||
|
|
||
| public interface EventClass<T> { |
There was a problem hiding this comment.
This should follow a "Mapper" concept that works on a generic and remove the static on the interface. I'm actually shocked that worked. But this is close, just make it <T> T map(...)
| } | ||
| } | ||
|
|
||
| public Class<? extends ExtrinsicError> mapErrorNameToErrorClass(String errorName, int moduleIndex){ |
There was a problem hiding this comment.
This is entirely unnecessary if you switch to the enum concept I mentioned above, you can do this all internal to that like we do in the CW
|
|
||
| Class<? extends EventClass<?>> eventClass = mapEventNameToEventClass(msaCreated.method()); | ||
|
|
||
| Object msaCreatedObject = eventClass.getMethod("create", Map.class).invoke(null, msaCreated.attributes()); |
There was a problem hiding this comment.
Mapper removes the need for reflection
| } | ||
|
|
||
| public static MsaCreated create(Map<String, RuntimeTypeDecoder.TypeAndValue> attributes) { | ||
| BigInteger msaIdValue = new BigInteger(attributes.get("MessageSourceId").getValue().toString()); |
There was a problem hiding this comment.
What is "MessageSourceId" cause this toString seems pretty dangerous
| public static MsaCreated create(Map<String, RuntimeTypeDecoder.TypeAndValue> attributes) { | ||
| BigInteger msaIdValue = new BigInteger(attributes.get("MessageSourceId").getValue().toString()); | ||
| MessageSourceId msaId = new MessageSourceId(msaIdValue); | ||
| ArrayList<Byte> byteList = (ArrayList<Byte>) ((HashMap) attributes.get("T::AccountId").getValue()).get("field1"); |
There was a problem hiding this comment.
You should just use List to avoid class cast exceptons if the impl changes. Also this field1 business is annoying. What type is this in the metadata file, is this a tuple of some kind?
| BigInteger msaIdValue = new BigInteger(attributes.get("MessageSourceId").getValue().toString()); | ||
| MessageSourceId msaId = new MessageSourceId(msaIdValue); | ||
| ArrayList<Byte> byteList = (ArrayList<Byte>) ((HashMap) attributes.get("T::AccountId").getValue()).get("field1"); | ||
| byte[] accountIdValue = new byte[byteList.size()]; |
There was a problem hiding this comment.
Look at https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/util/Collection.html#toArray(java.util.function.IntFunction) think that would be cleaner
No description provided.