Neptune-Crypto / neptune-core

anonymous peer-to-peer cash
Apache License 2.0
23 stars 7 forks source link

Failing CLI RPC call `wallet-status` #63

Open Sword-Smith opened 9 months ago

Sword-Smith commented 9 months ago

Calling wallet-status on my no9de with a balance of 95500 (version-4 alphanet) tokens results in this error:

$neptune-cli wallet-status 
Error: an error occurred while waiting for the server response

Caused by:
    frame size too big

I think the issue is a hard-coded limit to the size of RPC payloads. That limit should either be increased for the RPC, or the wallet-status should return other data than what it currently does. Currently the MsMembershipProofs are being returned for synced, unspent UTXOs. Maybe removing those MsMembershipProofs from this data would fix the issue? It's also possible we need to remove the Utxo data from this struct.

struct WalletStatus {
    pub synced_unspent_amount: Amount,
    pub synced_unspent: Vec<(WalletStatusElement, MsMembershipProof<Hash>)>,
    pub unsynced_unspent_amount: Amount,
    pub unsynced_unspent: Vec<WalletStatusElement>,
    pub synced_spent_amount: Amount,
    pub synced_spent: Vec<WalletStatusElement>,
    pub unsynced_spent_amount: Amount,
    pub unsynced_spent: Vec<WalletStatusElement>,
}

struct WalletStatusElement(pub u64, pub Utxo);

struct Utxo {
    pub lock_script_hash: Digest,
    pub coins: Vec<Coin>,
}

struct Coin {
    pub type_script_hash: Digest,
    pub state: Vec<BFieldElement>,
}
dan-da commented 8 months ago

Seems related to https://github.com/google/tarpc/issues/306. Although apparently a fix for that was made in tarpc 0.22 and we are presently using 0.33.0.

Also though, given #62 I wonder if this is worth investigating/fixing, as it seems specific to tarpc. If we move to json-rpc or some other language-agnostic rpc layer, then it likely won't be generating an error.

That said, it's generally best not to return/transmit unnecessary data, so my question would be: is there a reason that a client would need the MsMembershipProof and/or Utxo data when calling wallet-status? It could also be that we make this information optional, via parameter flag(s) or even by creating a 2nd RPC call such as wallet_status_details.

Sword-Smith commented 8 months ago

I think your suggestion of reducing the data returned (not returning the membership proofs) is the way to go. If we need them later, we'll add a new endpoint, like you suggest.