Skip to content

Enhance sui blockchain#2521

Merged
skudasov merged 7 commits intomainfrom
enhance-sui-blockchain
Apr 20, 2026
Merged

Enhance sui blockchain#2521
skudasov merged 7 commits intomainfrom
enhance-sui-blockchain

Conversation

@FelixFan1992
Copy link
Copy Markdown
Contributor

No description provided.

Copilot AI review requested due to automatic review settings April 16, 2026 19:17
@FelixFan1992 FelixFan1992 requested a review from a team as a code owner April 16, 2026 19:17
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 --json output, 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 ContractsDir optional when creating the Sui container, and include the created container in Output.

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.

Comment on lines +106 to +109
}

return nil, fmt.Errorf("failed to parse SuiWalletInfo from keytool output: %.200q", keyOut)
}
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a valid point, but do we care if we leak secrets used for tests?

Comment thread framework/components/blockchain/sui.go Outdated
Comment thread framework/components/blockchain/sui_test.go
Copy link
Copy Markdown
Contributor

@gustavogama-cll gustavogama-cll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

approving but would be good to have a stamp from @skudasov too

skudasov
skudasov previously approved these changes Apr 20, 2026
@skudasov skudasov merged commit 4949495 into main Apr 20, 2026
85 of 88 checks passed
@skudasov skudasov deleted the enhance-sui-blockchain branch April 20, 2026 20:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants