-
Notifications
You must be signed in to change notification settings - Fork 1
Client-VM Connector API #80
Comments
This generally looks good, but I have a few comments: // Finalizes the previous layer and initializes a new layer. Rewards are applied
// immediately at the start of the new layer.
// Returns an error if layerID != the VM's opinion of the next layer.
func (vm *VM) NewLayer(layerID types.LayerID, rewards map[types.Address]uint64) error We can't apply rewards at the start of a layer because it's impossible to predict which transactions will end up being applied (and thus what the total fee will be). OTOH, if you meant that rewards are applied at the start of the next layer, that's delaying them unnecessarily. I think rewards should be applied at the end of each layer - after the other transactions. I also think that this method should return the state root after applying the layer. This is in addition to On terminology: I would call this // Used in the rare case where full self healing requires that we revert state (and re-apply
// from a previous layer).
// Returns an error if the rewind fails, or if layerID >= the current top layer.
func (vm *VM) Rewind(layerID types.LayerID) (types.Hash32, error) It should be possible to call this with the current top layer in order to roll back any uncommitted changes. It should be mentioned that any uncommitted changes are always rolled back when this method is called, regardless of the layer provided (perhaps unless an error is returned, in which case we probably don't want anything to change). // Compares the ordering of two transactions from the same principal (based on nonce).
// Returns 1 if tx A should come first, -1 if tx B should come first, 0 if there is no ordering,
// and an error if either tx is syntactically invalid or if they do not have the same principal.
func (vm *VM) Compare(txdataA []byte, txdataB []byte) (int, error) Is this in order to abstract away the concept of a nonce from the node? I'm not sure it's enough. The mempool needs to be able to efficiently evict transactions with the same or lower nonce once a transaction is applied. If this method is all that's available then we'd need to compare with all transactions from the same principle that are currently in the mempool in order to know what to evict. It's possibly not the end of the world, but it would be more efficient if we had a linearized representation of the nonce that we can index based on and I think this is possible to achieve. Alternatively, we should let the node handle the nonce without abstracting it away. This mostly has disadvantages if we don't plan to implement the generalized nonce scheme before genesis, but I think that we actually do. Curios to hear other opinions on this. Additional note: the node will ultimately need to order transactions for the block, not only the mempool. It may prove to be confusing to have this ordering method and then apply additional ordering logic on top. Not sure about this, but I intuitively like having this logic entirely in the node. // Executes the transaction against the current state and returns a receipt.
// Called as part of optimistic transaction filtering, and when applying blocks from verified layers
// based on hare and/or tortoise output.
// Note that the transaction is an opaque byte array.
// Return error if the transaction is syntactically invalid.
func (vm *VM) ExecuteTransaction(txdata []byte) (types.Receipt, error) This should return an error if the transaction is contextually invalid, as well. Contextual invalidity is either a bad nonce value or insufficient balance to cover max gas. These checks should always be performed right before execution, regardless of any previous checks. I think that we should list the possible errors that this method returns (ideally we'd list the errors that all methods return, but I think this one is particularly important).
I think this flow is broken. It means that the new transactions don't get applied until the next layer is ready to be applied. That's also why I proposed to rename I think this also answers this question:
Since executing a block means executing all transactions and immediately calling Missing methodsI'm missing a method for getting a transaction's max balance transfer, which we need for the conservative balance calculation. Given how we want to handle nonces, max gas and gas price - there should be a method(s) that returns those as well. I think we should have a single method that returns nonce bytes, nonce mask, max gas, gas price and max balance transfer given a transaction. I think executing this method should be cheaper than executing |
made obsolete by current implementation spacemeshos/go-spacemesh#3220 |
Overview
Spacemesh maintains a strict abstraction boundary between node and VM. The node is responsible for networking, database, consensus, etc., but not for the contents, interpretation, or execution of transactions, nor for managing account state. The VM is responsible for the interpretation and execution of transactions and for managing account state.
This proposal specifies the API that a Spacemesh node should use to exchange data with the VM.
Goals and motivation
Maintaining a strict abstraction boundary between node and VM has multiple benefits:
The goal is to design the simplest possible API that facilitates this functionality. Note that, while the API is specified in golang in this proposal, it should be easy to translate into C types and functions for use over FFI and it should be easy to implement it in other languages.
High-level design
Under the account unification model (#49), the Spacemesh node is only responsible for low-level functionality such as networking, database, and consensus. The interpretation and execution of transactions and smart contracts, and management of account state, is handled entirely inside a modular VM. From the perspective of a node, a transaction is just an opaque byte array. The node therefore calls into the VM module to verify and execute transactions. When the node needs to be aware of some data contained in a transaction, such as the principal, nonce, or max gas (for purposes of selecting and ordering transactions), the VM layer unpacks these data and passes it back to the node.
Prior art
Proposed implementation
Note that the design explicitly excludes several things (some of which are implemented in the
svm
module in go-spacemesh today, and will need to be moved elsewhere):ApplyLayer()
: the node should callNewLayer()
(with layer rewards), followed byExecuteTransaction()
for each transaction in sequenceGetLayerApplied(txid)
: the VM does not store this information (it does not store transactions, only resultant state). The node should store it.GetTopLayer()
: the node should always know the top layer in state, and synchronizes this with the VM every time it callsNewLayer()
Rollback(txdata)
: we never roll back a single transaction. The VM internally rolls back state after attempting to apply an ineffective or failed transaction.ValidateNonceAndBalance()
: currently used in state projection, but no longer needed in account unification.VerifyTransaction()
should be used instead, as verification logic is entirely contained in the VM.Implementation plan
With respect to the
svm
module:mempool
module.HandleGossipTransaction
/HandleSyncTransaction
. These belong either in themempool
module or else in a new module that contains gossip handlers.ValidateNonceAndBalance
withVerifyTransaction
.Questions
ExecuteTransaction
need to callVerifyTransaction
internally? It probably does, since it cannot execute a transaction without verifying it first.VerifyTransaction
return only the principal, or the principal and the nonce? In other words, do we want a separateCompare
method, or do we want the nonce comparison logic to live in the node?Dependencies and interactions
Stakeholders and reviewers
Testing and performance
tbd
The text was updated successfully, but these errors were encountered: