Skip to content

check if bridge address has code before preparing tx payload#587

Merged
tclemos merged 6 commits intomainfrom
thiago/bridge-has-code
Apr 23, 2025
Merged

check if bridge address has code before preparing tx payload#587
tclemos merged 6 commits intomainfrom
thiago/bridge-has-code

Conversation

@tclemos
Copy link
Contributor

@tclemos tclemos commented Apr 21, 2025

@tclemos tclemos self-assigned this Apr 21, 2025
Copy link

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

The PR adds a check to ensure that the bridge address contains deployed code before preparing the transaction payload.

  • Adds a new helper function "hasCode" to validate the bridge address.
  • Integrates the "hasCode" check within "generateTransactionPayload" to return an error early if the bridge address is not valid.
Comments suppressed due to low confidence (1)

cmd/ulxly/ulxly.go:1118

  • Consider using a passed-in context (e.g., from generateTransactionPayload) instead of context.Background() to ensure consistent cancellation and timeout handling.
code, err := client.CodeAt(context.Background(), common.HexToAddress(address), nil)

@tclemos tclemos requested a review from Copilot April 21, 2025 19:36
Copy link

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 adds a safeguard to ensure that the bridge address has deployed code before preparing a transaction payload, addressing issue #184.

  • Introduced the hasCode function to check for contract code at a given address.
  • Integrated the hasCode check into generateTransactionPayload to prevent constructing a transaction with an invalid bridge address.

@tclemos tclemos requested a review from Copilot April 21, 2025 19:38
Copy link

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 adds a check to determine whether the provided bridge address actually contains deployed contract code before preparing the transaction payload.

  • Added the hasCode function which retrieves the bytecode at the provided address and returns an error if no code is detected.
  • Updated generateTransactionPayload to wrap and return an error if the bridge address has no contract code.

@tclemos tclemos requested a review from Copilot April 21, 2025 19:41
Copy link

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 ensures that the bridge address has on-chain code before preparing the transaction payload, mitigating potential issues with invalid addresses.

  • Added a helper function, hasCode, to check the existence of code at a given address.
  • Updated generateTransactionPayload to invoke hasCode and return an error if the bridge address lacks code.
Comments suppressed due to low confidence (1)

cmd/ulxly/ulxly.go:1117

  • [nitpick] Consider renaming 'hasCode' to something more descriptive like 'ensureCodePresent' to clarify that the function returns an error if no code is found.
func hasCode(ctx context.Context, client *ethclient.Client, address string) error {

@tclemos tclemos requested a review from Copilot April 21, 2025 19:45
Copy link

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 adds a pre-check to verify that the bridge address contains deployed code before preparing the transaction payload, addressing issue #184.

  • Added a helper function, ensureCodePresent, to check for code at a given address.
  • Integrated ensureCodePresent into generateTransactionPayload to prevent sending transactions to non-code addresses.
Comments suppressed due to low confidence (1)

cmd/ulxly/ulxly.go:1117

  • [nitpick] Consider adding unit tests to cover the ensureCodePresent function to verify its behavior when the code is absent at a given address.
func ensureCodePresent(ctx context.Context, client *ethclient.Client, address string) error {

Copy link
Member

@leovct leovct left a comment

Choose a reason for hiding this comment

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

lgtm, left a small comment

@tclemos tclemos requested a review from leovct April 22, 2025 14:29
@tclemos tclemos merged commit 0e61c78 into main Apr 23, 2025
13 checks passed
@tclemos tclemos deleted the thiago/bridge-has-code branch April 23, 2025 22:22
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.

3 participants