search

spacemeshos / go-spacemesh

Go Implementation of the Spacemesh protocol full node. 💾⏰💪
https://spacemesh.io
MIT License
752 stars 212 forks source link

Add transaction state to API #2961

Closed nkryuchkov closed 2 years ago

nkryuchkov commented 2 years ago

Description

According to https://github.com/spacemeshos/go-spacemesh/pull/2949#issuecomment-975034779:

in terms of TX life cycle below, i think we may need to push more events to make the account stream more complete. TX life cycle:

  • enters mempool: ReportNewTx is called with empty layer ID, in pending state
  • packed in a block received in mesh: AddBlockWithTxs is called but no TX related event were triggered. at this time we should be able to update that this TX were packed in a specific layer, but still in pending state
  • applied to a state: ReportNewTx is called with valid layer ID, in applied state.

so i'd prefer this PR to add a explicit TX state and determine the state via pushing more events, instead of simply fixing the syntax.

When API returns transaction details, the details don't include transaction state (e.g. pending, packed in a layer, applied), which may be useful

cc @countvonzero

avive commented 2 years ago

When API returns transaction details, the details don't include transaction state (e.g. pending, packed in a layer, applied), which may be useful.

which transaction-related api are you referring to as there are several?

On this topic, the transaction related api is quite hard to use, I wish we would revisit the design and provide a simpler way for clients to get transactions data. This is the current usage pattern by stageful clients: https://github.com/spacemeshos/product/blob/master/api_usage_patterns.md - least steps I could think about to get what Smapp needs while adhering to Api 0.2 design and current implantation w/o the transaction receipts. We should revisit the api design in regards to transactions and try to provide a much more streamlined and easy to use workflow for clients such as smapp. I think we were trying to be more pure and reflect a separation in the api between methods that return state information and methods that return mesh information depending on the service used but this leads to a not-so-easy-to-use pattern for users. I think we should seriously consider refactoring the whole tx related methods, possibly into the TransactionsService and reduce the number of different services, methods and streams that api clients such as wallets and explorer backends need to use in order to get full, accurate and current tx data.

@countvonzero @lrettig @moshababo

nkryuchkov commented 2 years ago

which transaction-related api are you referring to as there are several?

AccountMeshDataStream and TransactionsStateStream