Skip to content

fix(post-review): simplify to single REMOTE, template cleanup, CLA timing fix#3

Merged
aeddi merged 9 commits into
gnoverse:mainfrom
louis14448:fix/post-review-corrections
May 28, 2026
Merged

fix(post-review): simplify to single REMOTE, template cleanup, CLA timing fix#3
aeddi merged 9 commits into
gnoverse:mainfrom
louis14448:fix/post-review-corrections

Conversation

@louis14448
Copy link
Copy Markdown
Contributor

@louis14448 louis14448 commented May 27, 2026

Addresses the points left open by aeddi after merging #1, plus a follow-up simplification agreed with Rémi.

Changes

Single source of truth for account config (tests/samourai-crew/)

  • Mnemonics removed from Dockerfile, moved to Makefile as variables
  • Injected into the container at runtime via docker run -e
  • Rotating a key means one edit in one place

REMOTES replaced by single REMOTE

  • API simplified from a comma-separated RPC list to a single endpoint
  • Stress tests adapted: N wallets now bombard one REMOTE in parallel (wallet detection from keystore)
  • All files updated: Makefiles, funders, CI workflows, run_tests.sh, common.sh, README

CLA timing fix

  • sleep 2sleep 10 after CLA signing to let transactions commit before tests start
  • Fixes intermittent signature verification failed on e2e_nonce_replay

Template cleanup (_template/Makefile)

  • list-funding-* replaced by exit 1 stubs (forces contributors to implement)
  • Header comment: all account variables (addresses, mnemonics) must live in the Makefile, not the Dockerfile

README updated

  • Account injection pattern aligned (Makefile variables + -e flags)
  • REMOTE usage documented with shell example
  • New section: how to wire a new network into CI (funder script + workflow file + optional repo variable)

…ics, template cleanup

- Move mnemonics from Dockerfile to Makefile; inject via docker run -e
- Add REMOTES parsing explanation in template Makefile, samourai-crew Makefile, and README
- Replace opinionated list-funding-* in template with exit 1 stubs
- Remove ADDR_1/FUND_AMOUNT_* from template
- Document that all env vars (addresses, mnemonics) must be defined in the Makefile
- Update README steps 3/4/5 to reflect new account injection pattern
…mismatch

CLA transactions were still in the mempool when e2e_nonce_replay sent its
first tx, causing the node to reject it with signature verification failed
due to a stale sequence number on-chain.
Simplifies the contributor API from a comma-separated RPC list to a
single REMOTE variable. Stress tests now target one node with N wallets
in parallel instead of mapping one wallet to one RPC each.
@louis14448 louis14448 changed the title fix(post-review): single source of truth for accounts, REMOTES semantics, template cleanup fix(post-review): simplify to single REMOTE, template cleanup, CLA timing fix May 27, 2026
Copy link
Copy Markdown
Contributor

@aeddi aeddi left a comment

Choose a reason for hiding this comment

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

LGTM overall, please just address the last comments, then feel free to merge when done 👍

Comment thread _template/Makefile Outdated
Comment thread _template/Makefile Outdated
Comment thread _template/Makefile Outdated
Comment thread _template/Makefile Outdated
Comment thread funders/_template.sh Outdated
Comment thread README.md Outdated
Comment thread README.md Outdated
@louis14448 louis14448 force-pushed the fix/post-review-corrections branch from b899a94 to 27bd23a Compare May 27, 2026 22:28
@aeddi aeddi merged commit c056e57 into gnoverse:main May 28, 2026
1 of 2 checks passed
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