BlockchainTest: signature tampering tests pass on the hash check; Block.mine() is a no-op so signature verification is not tested

Cerrado
  • ¿Algo no está bien?
#19 0 comentarios 0 reacciones 0 asignados Ver en GitHub

Los mantenedores suelen responder en 2 días

Ya se ha fusionado un pull request relacionado.

  • #20 de @tanujb03 — fusionado

Evaluación

Dificultad
3/5
Tiempo estimado
1-2 días
Aptitud para principiantes
25/100
Tipo de issue
Error
Claridad
Bien especificado
Estado de actividad
Estancado
Stack tecnológico
java
Área
testing

Línea de trabajo

Start in src/test/java/minichain/BlockchainTest.java, at tamperedAmountInvalidatesChain() and forgedSenderInvalidatesChain(), and read Block.mine(int) and the hash check at the top of Blockchain.isValid(). Re-mine the tampered block against a fresh hash so the signature check is reached. Done when the tests still pass with the hash check bypassed and fail once the signature check in isValid() is commented out. PR #20 is already open against this issue, so check it before starting.

Escrito por el modelo de indexación a partir del texto del issue.

Descripción

good first issue tests

What I did

  1. Fresh checkout of main (commit 0e92205), ran tests with mvn test (all 18 pass).
  2. Checked the two tests in src/test/java/minichain/BlockchainTest.java that test tampering detection:
    • tamperedAmountInvalidatesChain()
    • forgedSenderInvalidatesChain()
  3. In both tests, after modifying a transaction in a block, the test calls: 
    tampered.mine(difficulty);
assertFalse(chain.isValid(), "Blockchain should be invalid because of tampered transaction");
  4. Examined Block.mine(int difficulty): 
    public void mine(int difficulty) {
    String target = "0".repeat(difficulty);
    while (!hash.startsWith(target)) {
        nonce++;
        hash = computeHash();
    }
}
  5. Tested mutation: in Blockchain.java, commented out the signature check: 
    // if (!tx.hasValidSignature()) return false;
    and ran mvn test.

What I expected
The README states:

"The system uses digital signatures to prevent forging transactions."

The tampering tests are intended to verify that digital signatures prevent forging transactions and detect altered amounts/senders even if someone re-mines the block to produce a valid proof-of-work hash. If signature verification is removed, the signature tampering tests should fail.

What happened instead
Even with signature verification completely deleted, both tests still pass (18/18 tests green).

Because the block was already mined, hash already starts with "0".repeat(difficulty). The loop condition !hash.startsWith(target) evaluates to false immediately on entry.
As a result:

  • Block.mine() exits without incrementing nonce or calling computeHash().
  • The block's stored hash remains the old hash from before tampering.
  • When chain.isValid() runs, it checks: 
    if (!current.getHash().equals(current.computeHash())) return false;
    and returns false right there because the stored hash does not match computeHash().
  • Execution never reaches the signature check loop at the bottom of isValid().

Additionally, in tamperedAmountInvalidatesChain(), the transaction modified (getTransactions().get(0)) is the mining reward transaction (from = "network"), which has no private key or signature check (hasValidSignature() returns true for network).

Environment

  • JDK 17 / 21, Maven
  • upstream/main (commit 0e92205)

I'd like to submit a PR to improve the test suite with proper re-mining so that signature verification is genuinely tested and isolated.

Lenguaje dominante
Java
Estrellas
0
Forks
3
Merge medio
2 d 5 h
PR fusionados (30 d)
15

Preparar el entorno

  • Sin Dockerfile ni archivo de Docker Compose
  • Tiene una plantilla de pull request
  • Sin guía de contribución

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Más de techcsispit/mini-chain

Todos los issues de techcsispit/mini-chain

Issues similares

Más issues de Java

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.