I try to made a new plugin for recover stats, and i need to have access to the block in OnPersist , but is not a parameter, now we can add the current block in this parameter, but maybe in the future we need other properties in other site, and is not a problem add this properties, but is a problem for maintain the plugins.
My proposal is to change all the plugins to receive one argument class, like OnPersistArguments when is persist, and we can extend this information in the future, like regular events does in .net.
Before:
public interface IP2PPlugin
{
bool OnP2PMessage(Message message);
bool OnConsensusMessage(ConsensusPayload payload);
}
After:
class MessageArguments
{
public Message Message{get;set;}
// You can add more properties
}
public interface IP2PPlugin
{
bool OnP2PMessage(object sender, MessageArguments args);
bool OnConsensusMessage(object sender, ConsensusMessageArguments args);
}
We can avoid sender
Also, we can change the logic of how the plugins works, and made events around the code, and if the plugin need it, you can attach to it. With this, we can avoid all plugins loops.
Before:
public class MyPlugin : Plugin, IP2PPlugin
{
public bool OnP2PMessage(Message message){ }
publi bool OnConsensusMessage(ConsensusPayload payload){ }
}
After:
public class MyPlugin : Plugin
{
public MyPlugin()
{
LocalNode.Singleton.OnP2PMessage+=OnP2PMessage;
}
public bool OnP2PMessage(Message message){ }
}
c#
public void OnPersist(Snapshot snapshot, IReadOnlyList<ApplicationExecuted> applicationExecutedList)
{
Block block = snapshot.PersistingBlock;
}
I think it is a good discussion and idea, @shargon.
We also detected the need for an event to monitor when a transaction enter in the mempool.
Thus, maybe we need to include something at OnNewTransaction and at RemoveOverCapacity().
What do you think?
In some applications it quite useful to monitor the time that a transaction enter the mempool of your node, as well as a notification if it is removed.
There is already a plugin I added for when things enter and leave the MemoryPool.
I really think that we should have only the interface IPlugin, we should be able to do anything attaching to events in whole neo, create a new kind of plugin for different things looks very weird to me
For me, this should be the best way for all plugins.
public class MyPlugin : Plugin
{
public MyPlugin()
{
LocalNode.Singleton.OnP2PMessage+=OnP2PMessage;
}
public bool OnP2PMessage(Message message){ }
}
@shargon, how can you specify the plugin type, if all inherit from same general Plugin class? I mean, how do you know which plugin methods are available, for each case? Which methods should be exposed on interface IPlugin?
Plugin doesn't need to expose any method, just is used for detect and load this object, then these plugins should attach to the event that it desire... Blockchain.OnNewBlock, MemoryPool.OnNewEntry, P2P.OnNewMessage , etc
So, in fact, the plugin will inject its own listened method into LocalNode, Blockchain, or other objects? Like registering a callback to itself?
For me this should be the way, using events, because you doesn't need to extend the plugins or create a new type of plugins, you only need to create events, and attach to these events.
Most helpful comment
I really think that we should have only the interface IPlugin, we should be able to do anything attaching to events in whole neo, create a new kind of plugin for different things looks very weird to me
For me, this should be the best way for all plugins.