penumbra - pclientd integration #480
Conversation
we can rather naively convert address strings to byte slices, and not have to worry about bech32m encoding: the protos will accept the raw byte slice as sufficient to reconstitute the address. sprinkled around some debug logging to make sense of what's breaking.
pd was *silently failing* to process the weirdly constructed csv file. the golang code was inserting liternal `\n`s rather than newlines. pd should definitely error on this. A previous debugging commit touched up the allocations indexing logic, which wasn't necessary to fix the bug, but sure made things more comprehensible. This commit restores the original allocations to the pre-debugging values, aiui.
Updates the GetBalanceByAddress request to handle a gRPC streaming request. Sadly the Penumbra docs aren't very clear that this method returns a stream; we'll work on that! For now, the for-loop-and-break-on-EOF seems a sufficiently idiomatic pattern for our needs. Still to come is making the balance info intelligble.
… hi/lo amount bytes
…igint and rpc changes
| } | ||
|
|
||
| fmt.Printf("STDOUT BAL: '%s'\n", string(stdout)) | ||
| return 0, nil |
There was a problem hiding this comment.
Can this be resolved with the implementation of (p *PenumbraClientNode) GetBalance(ctx context.Context, denom string)?
There was a problem hiding this comment.
I'm not 100% sure, they do technically return the same information the only difference being which piece of software is used to retrieve the balance (pclientd vs. pcli).
This PR moves away from making queries and broadcasting txs from the PenumbraAppNode in favor of the PenumbraClientNode when possible, so technically this method is unused now within interchaintest but it's not immediately clear to me if it will be used moving forward or if it's beneficial to expose methods on both objects.
I could remove this for now, change the method signature to accept a denom string and actually process the output properly so we can remove the todo, or open up an issue to come back to this later in a follow up PR. Thoughts?
| without this sleep statement we see intermittent failures where we will observe the tokens taken from alice's balance | ||
| but not added to bob's balance. after debugging it seems like this is because alice's client is in sync but bob's is not. | ||
| we may need a way to check if each client is in sync before making any assertions about chain state after some state transition. | ||
| alternatively, we may want to wrap penumbra related queries in a retry. |
There was a problem hiding this comment.
The newly added testutil.WaitForBlocksUtil method likely helps here
There was a problem hiding this comment.
great call out, this looks like it could be helpful. perhaps there is a way to check if a pclientd instance is in sync via a cmd that we can wrap in a function and pass into testutil.WaitForBlocksUtil
This PR introduces a basic integration with
pclientd, which is a standalone client daemon for interacting with private user data on Penumbra. We have ported over a network integration test from the Penumbra repo to assert that basic functionality viapclientdis working as intended.