Skip to content

Signed armageddon#945

Open
moran-abilea wants to merge 1 commit into
hyperledger:mainfrom
moran-abilea:signed-armageddon
Open

Signed armageddon#945
moran-abilea wants to merge 1 commit into
hyperledger:mainfrom
moran-abilea:signed-armageddon

Conversation

@moran-abilea

Copy link
Copy Markdown
Contributor

added --isSigned flag to armageddon load command.
added test as well

@moran-abilea moran-abilea self-assigned this Jun 22, 2026
@moran-abilea moran-abilea force-pushed the signed-armageddon branch 3 times, most recently from f68146d to b7b7bfc Compare June 23, 2026 08:49
@moran-abilea moran-abilea marked this pull request as ready for review June 23, 2026 08:49
@moran-abilea moran-abilea force-pushed the signed-armageddon branch 2 times, most recently from 9ebecc4 to 5c4da1a Compare June 23, 2026 09:28
@tock-ibm tock-ibm requested a review from Copilot June 23, 2026 11:25

Copilot AI left a comment

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.

Pull request overview

Adds support for sending signed transactions in the armageddon load command via a new --isSigned flag, and introduces an integration test that exercises signed load transactions and validates signatures in pulled blocks.

Changes:

  • Add --isSigned flag to armageddon load and plumb the option through load/send codepaths.
  • Create/load signing materials (MSP key + cert + org) and generate signed envelopes when enabled.
  • Add an integration test that runs armageddon load --isSigned and verifies received envelopes are properly signed.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
common/tools/armageddon/armageddon.go Adds --isSigned flag, loads signing materials, and sends signed envelopes when enabled.
testutil/tx/tx_utils.go Adds an envelope builder that can return signed vs unsigned envelopes.
testutil/signutil/signer.go Exposes MSP ID extraction helper for use by armageddon.
common/tools/armageddon/armageddon_test.go Adds integration test verifying signed load transactions are signed and verifiable.
docs/cli/armageddon.md Documents the new --isSigned CLI flag.
Comments suppressed due to low confidence (1)

common/tools/armageddon/armageddon.go:608

  • tx.PrepareEnvWithTimestampAndSignatureChoice can return nil (e.g., if signing fails). The current code will pass a nil envelope into SendTxToAllRouters, which is likely to panic. Handle a nil envelope explicitly and exit with an actionable error.
	for i := 0; i < numOfTxs; i++ {
		env := tx.PrepareEnvWithTimestampAndSignatureChoice(i, txSize, sessionNumber, isSigned, signer, certBytes, org)

		status := rl.GetToken()
		if !status {
			fmt.Fprintf(os.Stderr, "failed to send tx %d", i+1)
			os.Exit(3)
		}

		broadcastClient.SendTxToAllRouters(env)
	}

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread testutil/tx/tx_utils.go Outdated
Comment thread common/tools/armageddon/armageddon.go Outdated
Comment thread common/tools/armageddon/armageddon_test.go Outdated

parties := []types.PartyID{1, 2, 3, 4}
statusUnknown := common.Status_UNKNOWN
test_utils.PullFromAssemblers(t, &test_utils.BlockPullerOptions{

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.

You are using a different approach here.
The PullFromAssemblers is a method used in our integration tests.
Why not use the receive command as done in all armageddon tests ?

We want to confirm that armageddon works as expected, i.e., we can load signed transactions and receive them.

@moran-abilea moran-abilea Jun 23, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I thought that the test should also verify that each tx is really signed, but since we don't need to test it as part of this test i will change it to the simpler test approach instead

Comment thread common/tools/armageddon/armageddon_test.go Outdated
Comment thread common/tools/armageddon/armageddon.go Outdated
Comment thread testutil/tx/tx_utils.go Outdated
@moran-abilea moran-abilea force-pushed the signed-armageddon branch 7 times, most recently from d5345af to 3b88548 Compare June 29, 2026 11:01
Comment thread testutil/utils.go
Comment thread common/tools/armageddon/armageddon.go Outdated
loadTransactions *int
loadRate *string
loadTxSize *int
loadIsSigned *bool

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.

We need to support three modes:

  1. No signatures
  2. Full signatures — including the certificate
  3. Short signatures — using known certificates, referenced by hash

To prepare for this, let’s define a string flag, called "signature-mode," for example, with three meaningful values.

If the flag is missing from the command, it means no signatures should be used and the submitted transaction should remain unchanged from today (when txSize = 300, the size will refer to the full envelope size)

If the flag is set to full, the transaction is signed and includes the full certificate. This increases the transaction size. Therefore, when txSize = 300, the size will refer only to the payload data, not to the full envelope size.

If the flag is set to short, the transaction is signed and includes only the certificate hash. In this case, the transaction size is smaller than a transaction signed in full mode.

@moran-abilea moran-abilea force-pushed the signed-armageddon branch 3 times, most recently from 8565fd1 to 0d20085 Compare June 30, 2026 15:09
@moran-abilea moran-abilea force-pushed the signed-armageddon branch 7 times, most recently from 2dcb19b to 501c400 Compare June 30, 2026 16:10
@moran-abilea moran-abilea force-pushed the signed-armageddon branch 8 times, most recently from 13ea39e to 111fb9c Compare July 7, 2026 08:31
changing to use signed mode instead of boolean flag

Signed-off-by: Moran Abilea <moran.abilea@ibm.com>
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