Skip to content

use source of truth contract to derive addresses#3

Open
parkan wants to merge 3 commits intomasterfrom
feat/wagmi
Open

use source of truth contract to derive addresses#3
parkan wants to merge 3 commits intomasterfrom
feat/wagmi

Conversation

@parkan
Copy link
Collaborator

@parkan parkan commented Feb 2, 2026

  • reads from primary contract to derive addresses
  • CI test to bail on mismatch
  • more in-depth integration test (manual only right now)

echo "run 'go generate ./constants/...' and commit the result"
echo ""
echo "if using old addresses intentionally, add [skip-address-verify] to commit message"
exit 1
Copy link
Contributor

Choose a reason for hiding this comment

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

The generator and CI job will fail if the Glif RPC endpoints are unavailable during go generate. This could block CI for unrelated PRs if there's an RPC outage.

A few options to consider:

  • Add retry logic with backoff in the generator
  • Cache the last known good addresses_generated.go and only fail CI if addresses actually changed (vs just failing to fetch)
  • Mark the verify-addresses job as continue-on-error: true with a warning instead of failure on network errors

Not blocking, but worth considering for resilience.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

good point, I will make it continue if glif is down, it seems like the importance of this mechanism is still somewhat under debate upstream so I'm not going to overindex on it

fmt.Printf(" spregistry: %s\n", mainnet.SPRegistry.Hex())
fmt.Printf(" sessionkey: %s\n", mainnet.SessionKeyRegistry.Hex())

fmt.Println("reading calibration addresses...")
Copy link
Contributor

Choose a reason for hiding this comment

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

The 30-second timeout applies to the entire operation across both networks. If one RPC is slow, it could leave insufficient time for the second.

Consider either:

  • Using separate timeouts per network (e.g., 30s each)
  • Increasing the overall timeout to 60s to give more headroom

Minor concern - current timeout is probably fine under normal conditions.

@anjor
Copy link
Contributor

anjor commented Feb 3, 2026

other than the inlined comments lgtm (famous last words)

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.

2 participants