Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the Sui blockchain component to work more reliably across newer sui keytool generate --json output formats and Docker exec output shapes, while improving defaults for images/platforms and making contract mounting optional.
Changes:
- Add a robust parser for
sui keytool generate --jsonoutput, including Docker exec stream demuxing and support for compact JSON / preambles. - Update Sui defaults (image selection for arm64 vs non-arm64) and ensure Sui client config initialization before key generation.
- Make
ContractsDiroptional when creating the Sui container, and include the created container inOutput.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
framework/components/blockchain/sui.go |
Adds Docker exec demux + JSON parsing for keytool output; updates image defaults/platform handling; makes contract file mounting optional; sets Output.Container. |
framework/components/blockchain/sui_test.go |
Adds unit tests for the new keytool JSON parsing behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| return nil, fmt.Errorf("failed to parse SuiWalletInfo from keytool output: %.200q", keyOut) | ||
| } |
There was a problem hiding this comment.
The parse error includes a quoted slice of the full keyOut output, which (per SuiWalletInfo) contains sensitive material like the mnemonic. This risks leaking secrets into test logs/CI output when parsing fails. Consider removing raw output from the error message, or redacting sensitive fields before including any snippet (e.g., only include a small prefix/suffix without mnemonic).
There was a problem hiding this comment.
this is a valid point, but do we care if we leak secrets used for tests?
gustavogama-cll
left a comment
There was a problem hiding this comment.
approving but would be good to have a stamp from @skudasov too
7cc1c3d
No description provided.