chia-blockchain
chia-blockchain copied to clipboard
Wallet protocol update
Purpose:
The wallet protocol is still fairly limited in functionality.
This PR adds a lot of really nice to have features to it, and enables various things wallets could not previously do.
Existing Protocol Limitations:
- You cannot unsubscribe from puzzle hashes or coin ids without disconnecting from the peer completely, so once you reach the limit you have to reset everything
- If a puzzle hash or coin id has too many coins to return in one message, the response will be truncated (which is not a reasonable outcome for some use cases)
- You cannot request coin state without subscribing to the coin id, which is unnecessary and means the subscription limit will be reached faster
- If a puzzle reveal or solution exceeds the maximum message size when serialized, there is no way to request it
- It is not currently possible to ask a node for information about the mempool status
- There is no way to know which transactions belong to your wallet without downloading them all and checking by hand
New Features
- Removing subscriptions
- Requesting state without subscribing
- Listening to the mempool
- Filtering coin states by amount, hint, and spent status
- Pagination when syncing coin state for puzzle hashes
- Mempool fee estimation utilities
- Request puzzle and solutions with support for backrefs
Proposed New Protocol
Note that transaction_id
in the below messages refers to the "spend bundle name", which is calculated by hashing a spend bundle.
This is different from a wallet's random or sequential transaction id, and is what is used to identify transactions in the mempool.
Add Puzzle Subscriptions
class RequestAddPuzzleSubscriptions:
puzzle_hashes: List[bytes32]
class RespondAddPuzzleSubscriptions:
puzzle_hashes: List[bytes32]
height: uint32
header_hash: bytes32
Adds puzzle hashes to the subscription list until the limit has been reached, returning the hashes that were actually added. If the limit is exceeded, the wallet can either bail or subscribe with other nodes instead. Also gives the current peak height and header hash.
Note that unlike RegisterForPhUpdates
, this does not return any coin states.
There is a RequestPuzzleState
message which fetches the current coin states instead.
The idea of this is to enable you to subscribe only, request current state only, or do both at the same time.
Add Coin Subscriptions
class RequestAddCoinSubscriptions:
coin_ids: List[bytes32]
class RespondAddCoinSubscriptions:
coin_ids: List[bytes32]
height: uint32
header_hash: bytes32
Adds coin ids to the subscription list until the limit has been reached, returning the ids that were actually added. If the limit is exceeded, the wallet can either bail or subscribe with other nodes instead. Also gives the current peak height and header hash.
Note that unlike RegisterForCoinUpdates
, this does not return any coin states.
There is a RequestCoinState
message which fetches the current coin states instead.
The idea of this is to enable you to subscribe only, request current state only, or do both at the same time.
Add Transaction Subscriptions
class RequestAddTransactionSubscriptions:
transaction_ids: List[bytes32]
class RespondAddTransactionSubscriptions:
transaction_ids: List[bytes32]
Adds transaction ids to the subscription list, until the limit has been reached, returning the ids that were actually added. If the limit is exceeded, the wallet can either bail or subscribe with other nodes instead.
This only subscribes to mempool updates, which are not able to be validated since they have not been included in the blockchain. You could receive updates from multiple peers and compare them, determining if you are being spammed or misled by a peer and ban it.
Remove Puzzle Subscriptions
class RequestRemovePuzzleSubscriptions:
puzzle_hashes: Optional[List[bytes32]]
class RespondRemovePuzzleSubscriptions:
puzzle_hashes: List[bytes32]
Removes puzzle hashes from the subscription list (or all of them if None
), returning the hashes that were actually removed.
Remove Coin Subscriptions
class RequestRemoveCoinSubscriptions:
coin_ids: Optional[List[bytes32]]
class RespondRemoveCoinSubscriptions:
coin_ids: List[bytes32]
Removes coin ids from the subscription list (or all of them if None
), returning the ids that were actually removed.
Remove Transaction Subscriptions
class RequestRemoveTransactionSubscriptions:
transaction_ids: Optional[List[bytes32]]
class RespondRemoveTransactionSubscriptions:
transaction_ids: List[bytes32]
Removes transaction ids from the subscription list (or all of them if None
), returning the ids that were actually removed.
Request Puzzle State
class RequestPuzzleState:
puzzle_hashes: List[bytes32]
previous_height: uint32
header_hash: bytes32
max_height: Optional[uint32]
filters: CoinStateFilters
subscribe_when_finished: bool
class RespondPuzzleState:
puzzle_hashes: List[bytes32]
height: uint32
header_hash: bytes32
is_finished: bool
coin_states: List[CoinState]
class RejectPuzzleState:
pass
class CoinStateFilters:
include_spent: bool
include_unspent: bool
include_hinted: bool
min_amount: uint64
Requests coin states that match the given puzzle hashes (or hints).
Unlike RegisterForPhUpdates
, this does not add subscriptions for the puzzle hashes automatically.
When subscribe_when_finished
is set to True
, it will add subscriptions, but only once the last batch has been returned.
As well as this, previously it was impossible to get all coin records if the number of items exceeded the limit.
This implementation allows you to continue where you left off with previous_height
and header_hash
.
If a reorg of relevant blocks occurs while doing so, previous_height
will no longer match header_hash
.
This can be handled by a wallet by simply backtracking a bit, or restarting the download from zero. It could be inconvenient, but at least you can detect it.
In the event that a reorg is detected by a node, RejectPuzzleState
will be returned. This is the only scenario it will be rejected directly like this.
Additionally, it is now possible to filter out spent, unspent, or hinted coins, as well as coins below a minimum amount. This can reduce the risk of spamming or DoS of a wallet in some cases, and improve performance.
Request Coin State
class RequestCoinState:
coin_ids: List[bytes32]
previous_height: uint32
header_hash: bytes32
max_height: Optional[uint32]
subscribe: bool
class RespondCoinState:
coin_ids: List[bytes32]
coin_states: List[CoinState]
class RejectCoinState:
pass
Request coin states that match the given coin ids.
Unlike RegisterForCoinUpdates
, this does not add subscriptions for the coin ids automatically.
When subscribe
is set to True
, it will add and return as many coin ids to the subscriptions list as possible.
Unlike the new RequestPuzzleState
message, this does not implement batching for simplicity.
However, you can still specify the previous_height
and header_hash
to start from.
If a reorg of relevant blocks has occurred, previous_height
will no longer match header_hash
.
This can be handled by a wallet depending on the use case (for example by restarting from zero). It could be inconvenient, but at least you can detect it.
In the event that a reorg is detected by a node, RejectCoinState
will be returned. This is the only scenario it will be rejected directly like this.
Transaction Added
class TransactionAddedUpdate:
transaction_id: bytes32
height_added: uint32
cost: uint64
additions: List[Coin]
removals: List[Coin]
Sent to wallet peers using the new protocol version when a transaction matching any subscription is added to the mempool.
Transaction Removed
class TransactionRemovedUpdate:
transaction_id: bytes32
height_removed: uint32
inclusion_header_hash: Optional[bytes32]
additions: List[Coin]
removals: List[Coin]
Sent to wallet peers using the new protocol version when a transaction matching any subscription is removed from the mempool.
The inclusion_header_hash
is None
when it's removed from the mempool but not included in a block.
Otherwise, it's the header hash of height_removed
, and the block that the transaction was included in.
Request Puzzle Solution Backrefs
class RequestPuzzleSolutionBackrefs:
coin_id: bytes32
height: uint32
class RespondPuzzleSolutionBackrefs:
puzzle_reveal: Bytes
solution: Bytes
Unlike RequestPuzzleSolution
, this will return the serialized puzzle and solution with backrefs.
It's not clear to me how this would work when the program reaches into previous blocks via compression, so more thought is needed.
Request Cost Info
class RequestCostInfo:
pass
class RespondCostInfo:
max_block_cost: uint64
max_mempool_cost: uint64
current_mempool_cost: uint64
estimated_total_fee: uint64
This is a rough draft, but essentially the wallet protocol should be able to get information needed to better estimate fees.
Concerns
The data which a full node gives you cannot be trusted unless it is your own node. These additions to the protocol make no effort to make untrusted data any easier to validate. The best method I am aware of is still just to cross reference with other peers and ban outliers.
Checklist (WIP)
Coin Subscriptions & State
- [x] Waiting for https://github.com/Chia-Network/chia-blockchain/pull/17300 to be merged.
- [x] New messages for adding and removing subscriptions.
- [x] New messages for fetching coin state without subscribing.
- [x] Synced coin states for puzzle hashes can be filtered and sent in batches.
- [x] Update serialization and regression tests with these messages.
- [ ] Write unit tests for these subscription and coin state protocol messages.
- [ ] Write tests for syncing a wallet from zero using this new sync protocol.
- [ ] Write tests for syncing from the last known height, and recovering when a reorg occurs.
- [ ] Write tests for a reorg occurring during a wallet's sync.
Transaction Subscriptions
- [ ] New messages for adding and removing transaction subscriptions.
- [ ] New message(s) for transaction status updates.
- [ ] Implement transaction subscriptions, using the local mempool.
- [ ] Update serialization and regression tests with these messages.
- [ ] Write unit tests for these subscription protocol messages.
- [ ] Write tests for what happens when a reorg occurs.
Before Merge
- [ ] Ensure protocol version is correct, and doesn't need to be bumped again.
- [ ] Implement protocol messages in Rust https://github.com/Chia-Network/chia_rs/pull/369 to keep parity.
Pull Request Test Coverage Report for Build 7615663424
Warning: This coverage report may be inaccurate.
We've detected an issue with your CI configuration that might affect the accuracy of this pull request's coverage report. To ensure accuracy in future PRs, please see these guidelines. A quick fix for this PR: rebase it; your next report should be accurate.
- 0 of 0 changed or added relevant lines in 0 files are covered.
- No unchanged relevant lines lost coverage.
- Overall coverage decreased (-0.05%) to 90.547%
Totals | |
---|---|
Change from base Build 7574540476: | -0.05% |
Covered Lines: | 95132 |
Relevant Lines: | 105023 |
💛 - Coveralls
Conflicts have been resolved. A maintainer will review the pull request shortly.
In general, there are opportunities to improve the protocol design as new messages are added. Subscriptions essentially open a "channel" multiplexed over the message stream, so we could introduce an explicit channel concept - just an incrementing nonce namespaced on each websocket.
The request could allow specifying a start block height, so that we don't sync from genesis each time. Wallets can track a "current height" per puzzle hash.
Perhaps something like:
# non-message struct
class PuzzleSubscriptionRequest:
puzzle_hash: bytes32
height: uint32
# message
class RequestAddPuzzleSubscriptions:
requests: List[PuzzleSubscriptionRequest]
# non-message struct
class RespondSubscriptionRequest:
channel: uint32
puzzle_hash: bytes32
height: uint32
header_hash: bytes32
# message
class RespondAddPuzzleSubscriptions:
requests: List[RespondSubscriptionRequest]
This makes unsubscribe simpler:
class RequestRemoveSubscriptions:
channel: List[uint32]
class RespondRemoveSubscriptions:
channel: List[uint32]
Update messages might look something like:
class SubscriptionUpdate:
channel: uint32
updates: list[(info about coin spend)]
completed_updated_to_height: uint32
The completed_updated_to_height
field is a height that we have completely exhausted all updates for. This might be much larger than the block of the last update (if we see no further spends for example) or ONE LESS than the block the last update occurs in (if there are further updates in the same block that will not fit into this message). Essentially, it's the value the wallet can later subscribe to and be assured to have not missed an update.
Generally speaking, having multiple potential responses to a particular request is annoying. It means when a client makes a request, it has multiple message IDs it needs to wait on to find a response.
The serialization protocol doesn't support enum
types, and there's no obvious way to see how to do it. However, we could simply have each different potential response be Optional
. As an example, rather than having two messages each with their own id:
class RespondPuzzleState:
puzzle_hashes: List[bytes32]
next_height: Optional[uint32]
next_header_hash: Optional[bytes32]
coin_states: List[CoinState]
class RejectPuzzleState:
header_hash: Optional[bytes32]
we could merge them:
# used in `RespondPuzzleState`
class ResponsePuzzleStateOK:
puzzle_hashes: List[bytes32]
next_height: Optional[uint32]
next_header_hash: Optional[bytes32]
coin_states: List[CoinState]
# used in `RespondPuzzleState`
class ResponsePuzzleStateFail:
header_hash: Optional[bytes32]
# message
class RespondPuzzleState:
ok: Optional[ResponsePuzzleStateOK]
fail: Optional[ResponsePuzzleStateFail]
Here, exactly one of ok
and fail
should be not None
. We can't enforce this through type-checking, but we can enforce at runtime, and simply fail on messages that don't comply like we would any other ill-formed message (for example, disconnect from or otherwise punish the remote).
In general, there are opportunities to improve the protocol design as new messages are added. Subscriptions essentially open a "channel" multiplexed over the message stream, so we could introduce an explicit channel concept - just an incrementing nonce namespaced on each websocket.
The request could allow specifying a start block height, so that we don't sync from genesis each time. Wallets can track a "current height" per puzzle hash.
Perhaps something like:
# non-message struct class PuzzleSubscriptionRequest: puzzle_hash: bytes32 height: uint32 # message class RequestAddPuzzleSubscriptions: requests: List[PuzzleSubscriptionRequest] # non-message struct class RespondSubscriptionRequest: channel: uint32 puzzle_hash: bytes32 height: uint32 header_hash: bytes32 # message class RespondAddPuzzleSubscriptions: requests: List[RespondSubscriptionRequest]
This makes unsubscribe simpler:
class RequestRemoveSubscriptions: channel: List[uint32] class RespondRemoveSubscriptions: channel: List[uint32]
Update messages might look something like:
class SubscriptionUpdate: channel: uint32 updates: list[(info about coin spend)] completed_updated_to_height: uint32
The
completed_updated_to_height
field is a height that we have completely exhausted all updates for. This might be much larger than the block of the last update (if we see no further spends for example) or ONE LESS than the block the last update occurs in (if there are further updates in the same block that will not fit into this message). Essentially, it's the value the wallet can later subscribe to and be assured to have not missed an update.
I don't understand the benefit of this, it seems like a more complicated API to do roughly the same thing. But maybe I'm misunderstanding something? Subscribing doesn't actually return any coin states, it just registers that you're interested in a given set of puzzle hashes going forward whenever coin states are updated.
The RequestPuzzleState
message allows you to specify the height and header hash to start from, like you're describing, and you keep requesting more coin states until you've reached the peak or you decide to stop.
Generally speaking, having multiple potential responses to a particular request is annoying. It means when a client makes a request, it has multiple message IDs it needs to wait on to find a response.
The serialization protocol doesn't support
enum
types, and there's no obvious way to see how to do it. However, we could simply have each different potential response beOptional
. As an example, rather than having two messages each with their own id:class RespondPuzzleState: puzzle_hashes: List[bytes32] next_height: Optional[uint32] next_header_hash: Optional[bytes32] coin_states: List[CoinState] class RejectPuzzleState: header_hash: Optional[bytes32]
we could merge them:
# used in `RespondPuzzleState` class ResponsePuzzleStateOK: puzzle_hashes: List[bytes32] next_height: Optional[uint32] next_header_hash: Optional[bytes32] coin_states: List[CoinState] # used in `RespondPuzzleState` class ResponsePuzzleStateFail: header_hash: Optional[bytes32] # message class RespondPuzzleState: ok: Optional[ResponsePuzzleStateOK] fail: Optional[ResponsePuzzleStateFail]
Here, exactly one of
ok
andfail
should be notNone
. We can't enforce this through type-checking, but we can enforce at runtime, and simply fail on messages that don't comply like we would any other ill-formed message (for example, disconnect from or otherwise punish the remote).
We don't have any precedent for doing it this way, and this increases the size of the protocol messages and requires differentiating between the response types just the same. Message responses are identified by an id, so rather than listening for a given response type you just wait until you receive a message with the same nonce as was passed in the request.
It's pretty simple to do, here is an example I have (a bit outdated, but it shows the idea): https://github.com/Chia-Network/chia_rs/pull/369/files#diff-7f65462d1a9e187413a5013428d70e00a201bec3b2d7e0b26ce7192cbc281462R275
You will usually use RequestPuzzleState
and RequestCoinState
as an entrypoint for subscribing, since it's more resilient to reorgs and more concise. There's a case to be made for removing the AddSubscriptions
messages entirely if they don't have a use-case.
We don't have any precedent for doing it this way, and this increases the size of the protocol messages and requires differentiating between the response types just the same. Message responses are identified by an id, so rather than listening for a given response type you just wait until you receive a message with the same nonce as was passed in the request.
I did notice a Optional[uint16]
in the wrapper Message
type. I thought I tried using that and it was None
, but I will try again. It's definitely the best way I can think of to route messages, by mapping the responses to their requests with a nonce.
I don't understand the benefit of this, it seems like a more complicated API to do roughly the same thing. But maybe I'm misunderstanding something? Subscribing doesn't actually return any coin states, it just registers that you're interested in a given set of puzzle hashes going forward whenever coin states are updated.
For request/responses, I see the signature conceptually as
class SomeRemoteObject:
async def some_request(self, foo: SerializableType1, bar: SerializableType2) -> SerializableType3:
...
For subscriptions, I see it as
class SomeRemoteObject:
async def subscribe_to_something(self, identifier: SerializableIdentifier) -> AsyncIterator[SubscriptionUpdate]:
...
Internal to this, you'd use a nonce to identify subscriptions. Remote messages containing subscription updates could use that subscription nonce to route it to the correct async iterator, and also use that nonce to inform the remote to cancel the subscription.
This makes local use really easy. You get the async iterator with the subscription api, then simply await
to get items out.
[As I write this, I realize you probably actually want to return an async context manager so you can more affirmatively cancel the subscription with a remote message no matter how you exit locally.]
I realize these comments aren't really about the code. But I've been trying to write code to connect to remote chia nodes and I've found it to be much more complicated than it should be, and how certain changes could allow for better abstractions and smaller code. But maybe you don't want to tackle this now.
We don't have any precedent for doing it this way, and this increases the size of the protocol messages and requires differentiating between the response types just the same. Message responses are identified by an id, so rather than listening for a given response type you just wait until you receive a message with the same nonce as was passed in the request.
I did notice a
Optional[uint16]
in the wrapperMessage
type. I thought I tried using that and it wasNone
, but I will try again. It's definitely the best way I can think of to route messages, by mapping the responses to their requests with a nonce.
It should be the same value as the request id, if that's left as None the response will be too. If it's an update (ie CoinStateUpdate), it will also be None.
Btw I wrote the chia-client
crate in this repo which helps connect to (trusted) full node peers and routes messages. It's not perfect (lacking proper error handling, disconnect handling, and validation) but it's a start.
Generally speaking, having multiple potential responses to a particular request is annoying. It means when a client makes a request, it has multiple message IDs it needs to wait on to find a response.
my recollection of how our protocol driver works is that it's not waiting for message IDs, it's demultiplexing based on transaction IDs (or request IDs). So, any message can (in theory) be a response to any other. The current protocols usually have one success message and a separate failure message too.
@richardkiss it sounds like the general features of the protocol you're describing matches what @Rigidity has documented, and is implementing.
Some major differences is that in his protocol you subscribe and request coin states in bulk, with many coin IDs and puzzle hashes. Also, subscriptions aren't given shorter names (but are just the coin ID or puzzle hash). But the updates are still delivered to the client in block-height order, making it possible for the wallet to be interrupted and resuming from the height it left off.
We don't have any precedent for doing it this way, and this increases the size of the protocol messages and requires differentiating between the response types just the same. Message responses are identified by an id, so rather than listening for a given response type you just wait until you receive a message with the same nonce as was passed in the request.
I did notice a
Optional[uint16]
in the wrapperMessage
type. I thought I tried using that and it wasNone
, but I will try again. It's definitely the best way I can think of to route messages, by mapping the responses to their requests with a nonce.It should be the same value as the request id, if that's left as None the response will be too. If it's an update (ie CoinStateUpdate), it will also be None.
Your postman doesn't need to open an envelope to deliver it. Our envelope is chia.server.outbound_message.Message
with the three fields, type
, id
and data
. The type
and id
work together to determine where this message should be routed. (IMO, it's a bit silly to use both of these fields for this purpose, but this would be pretty tough to change at this point.)
Some major differences is that in his protocol you subscribe and request coin states in bulk, with many coin IDs and puzzle hashes. Also, subscriptions aren't given shorter names (but are just the coin ID or puzzle hash). But the updates are still delivered to the client in block-height order, making it possible for the wallet to be interrupted and resuming from the height it left off
What do the updates actually look like? I don't see that in this discussion. Are we using the existing messages to do this?
Some major differences is that in his protocol you subscribe and request coin states in bulk, with many coin IDs and puzzle hashes. Also, subscriptions aren't given shorter names (but are just the coin ID or puzzle hash). But the updates are still delivered to the client in block-height order, making it possible for the wallet to be interrupted and resuming from the height it left off
What do the updates actually look like? I don't see that in this discussion. Are we using the existing messages to do this?
RespondPuzzleState is used to respond to incremental requests for the initial state. CoinStateUpdate is sent every block or when a reorg occurs to give you changes for things you're subscribed to. I do not intend to drastically change how the protocol works, but rather to add missing functionality and lift some restrictions. Changing how protocol messages in general work seems pretty out of scope to me.
We don't have any precedent for doing it this way, and this increases the size of the protocol messages and requires differentiating between the response types just the same. Message responses are identified by an id, so rather than listening for a given response type you just wait until you receive a message with the same nonce as was passed in the request.
I did notice a
Optional[uint16]
in the wrapperMessage
type. I thought I tried using that and it wasNone
, but I will try again. It's definitely the best way I can think of to route messages, by mapping the responses to their requests with a nonce.It should be the same value as the request id, if that's left as None the response will be too. If it's an update (ie CoinStateUpdate), it will also be None.
Your postman doesn't need to open an envelope to deliver it. Our envelope is
chia.server.outbound_message.Message
with the three fields,type
,id
anddata
. Thetype
andid
work together to determine where this message should be routed. (IMO, it's a bit silly to use both of these fields for this purpose, but this would be pretty tough to change at this point.)
It seems reasonable to me, they mean different things. The unique nonce (id) identifies which message it is, and the type identifies which kind of response you're receiving (ie a success or a failure). If you receive anything you don't expect, you ban the peer.
This pull request has conflicts, please resolve those before we can evaluate the pull request.
Conflicts have been resolved. A maintainer will review the pull request shortly.
This is ready for initial reviews, though rate limits have not been set, so it is not ready for merge yet. That is what I will work on next.
My understanding was that there was consensus (or at least some agreement) on altering the protocol to essentially merge the request for subscription with requesting coin states. and the subscription would only kick in if the complete state could be returned. Otherwise the wallet was expected to request the next range of coin states.
RequestPuzzleState
syncs batch by batch until it reaches the peak and subscribes from there on, and RequestAddPuzzleSubscriptions
directly adds the subscriptions without requesting initial coin data and returns the current peak as of when you subscribed.
I can get rid of RequestAddPuzzleSubscriptions
, but I do like the flexibility having it offers you if you want it. And having both Add
/Remove
made logical sense to me. Though, I think you could technically get the same behavior by just calling RequestPuzzleState
with CoinStateFilters(False, False, False, 0)
to avoid getting any coins and immediately subscribe. So maybe it would just be unnecessary to have it. You could also just call RegisterForPhUpdates
and ignore the response I suppose.
So I guess it's technically unnecessary?
I can get rid of RequestAddPuzzleSubscriptions, but I do like the flexibility having it offers you if you want it.
Perhaps I'm missing something. Given that the wallet can't know which block will be the node's peak (by the time it receives the request) I question the usefulness of being able to just subscribe. You can't know what you're getting up front. You do see the height and block hash in the subscription response, so you do know what you ended up with.
This pull request has conflicts, please resolve those before we can evaluate the pull request.
Conflicts have been resolved. A maintainer will review the pull request shortly.
I have also updated CHIP-0026 according to the new changes.
I think we should either get rid of the duplicated check above if it's not important enough, or add # pragma: no cover
to it. Since it's an almost impossible scenario to be in imo (and I don't know how we'd cover it with tests).
This pull request has conflicts, please resolve those before we can evaluate the pull request.
Conflicts have been resolved. A maintainer will review the pull request shortly.
I think it's reasonable to not cover the checks for race condition
File | Coverage | Missing Lines |
---|---|---|
chia/full_node/full_node_api.py |
98.2% | lines 1824, 1881 |
Total | Missing | Coverage |
---|---|---|
818 lines | 2 lines | 99% |