Revisar um PR que movimenta dinheiro é diferente de revisar uma tela de cadastro. O erro costuma ser silencioso, aparece dias depois na conciliação e às vezes não tem volta, porque o Pix já saiu. Por isso eu uso um checklist fixo com oito pontos: tipo e arredondamento de valores, idempotência, concorrência, posição das transações, transições de estado, estornos, dados sensíveis nos logs e testes de caminho de falha. Saber aplicar isso com rapidez e sem travar o time é uma das habilidades que mais separam um dev pleno de um sênior em sistemas financeiros.

Este texto é o checklist que fui montando trabalhando com conta digital, maquininha, split e integrações com Pagar.me, Stripe, Asaas e Zoop. Não é teoria de code review. É o que eu realmente procuro no diff.

Por que code review de código financeiro é diferente?

Em código comum, um bug gera um ticket. Em código financeiro, ele gera um prejuízo, um cliente com saldo errado ou um repasse duplicado que você vai ter que cobrar de volta de um lojista. Três características mudam a forma de revisar:

  1. Irreversibilidade. Uma transferência liquidada não tem rollback. O banco de dados até volta, mas o dinheiro não volta.
  2. Bugs que só aparecem sob carga ou falha. A maioria dos problemas graves não quebra nenhum teste feliz. Só aparece quando o gateway dá timeout, quando chegam dois webhooks iguais ou quando dois requests batem no mesmo saldo ao mesmo tempo.
  3. Erro distribuído no tempo. Um centavo perdido por arredondamento não aparece em nenhum alerta. Aparece no fechamento do mês, multiplicado pelo volume.

Isso quer dizer que o revisor não pode ler o diff só perguntando "isso funciona?". A pergunta certa é "o que acontece quando isso roda duas vezes, ao mesmo tempo, ou pela metade?".

Qual checklist usar para revisar um PR que mexe com dinheiro?

Antes de entrar em cada item, aqui está a lista completa. Eu passo por ela na ordem, porque os primeiros itens são os mais baratos de verificar e os que mais aparecem.

  1. Valores estão em inteiros (centavos) e o arredondamento é explícito?
  2. Toda operação com efeito externo é idempotente?
  3. Existe leitura-modificação-escrita sem lock?
  4. Jobs, eventos e chamadas externas estão na posição certa em relação à transação?
  5. Mudanças de status respeitam as transições permitidas?
  6. Estornos parciais e splits fecham a conta até o último centavo?
  7. Logs e payloads salvos expõem dados sensíveis?
  8. Os testes cobrem falha, duplicidade e ordem trocada?

Como identificar problemas de arredondamento e tipo de valor no review?

É o item mais simples e o que mais encontro. Qualquer float, round() solto ou coluna decimal sendo manipulada como número de ponto flutuante no PHP é um alerta.

// Sinal de alerta no diff
$fee = $amount * 0.0399;
$net = round($amount - $fee, 2);

O problema não é só a imprecisão do ponto flutuante. É que a regra de arredondamento fica implícita. Ninguém decidiu se a taxa arredonda para cima, para baixo ou meio para cima, e quem paga a diferença. Em review, eu peço que o valor trafegue em centavos como inteiro e que a regra apareça no código:

// Valores em centavos (int); taxa em pontos-base (399 = 3,99%)
$feeCents = intdiv($amountCents * 399 + 5000, 10000); // half-up para valores positivos
$netCents = $amountCents - $feeCents;

Se o domínio já usa uma biblioteca como brick/money, melhor ainda: a pergunta de review vira "por que este trecho saiu da biblioteca?". Também confiro se a soma das partes bate com o total. Taxa mais líquido tem que ser igual ao bruto, sempre, por construção, e não por coincidência.

Como verificar idempotência e retries durante o code review?

Idempotência é a propriedade de uma operação produzir o mesmo efeito se executada uma ou várias vezes. Em pagamentos, ela é obrigatória porque retry é inevitável: o Horizon vai reexecutar o job, o gateway vai reenviar o webhook, o usuário vai clicar duas vezes.

No diff, eu procuro três coisas:

  1. Jobs com $tries maior que 1 que chamam API de pagamento. Se o job deu timeout depois que o provedor processou a cobrança, o retry cobra de novo. A chamada precisa de uma chave de idempotência estável, gerada antes e persistida com o registro, e não criada dentro do handle().
  2. Unicidade garantida pelo banco, não por if. Um exists() seguido de create() não protege contra duas execuções simultâneas. Quero ver um índice único.
  3. Webhooks registrando o ID do evento antes de aplicar o efeito. Esse tema eu detalhei no post sobre como processar webhooks de pagamento duplicados e fora de ordem uma única vez.
// A chave nasce junto com o payout, não no momento da chamada
$response = Http::withHeaders([
        'Idempotency-Key' => $payout->idempotency_key,
    ])
    ->timeout(10)
    ->post($endpoint, $payload);

Nem todo provedor aceita um cabeçalho de idempotência. Quando não aceita, a pergunta de review muda: como esse código descobre, antes de tentar de novo, se a operação anterior já foi feita? Normalmente a resposta é consultar pelo identificador externo que você mesmo enviou.

Como encontrar race condition em um PR de saldo?

Race condition é quando o resultado depende da ordem em que duas execuções concorrentes intercalam. O padrão clássico no diff é ler um valor, calcular em PHP e salvar:

// Duas requisições simultâneas leem o mesmo saldo e uma sobrescreve a outra
$account = Account::find($id);
$account->balance -= $amountCents;
$account->save();

O que eu peço no review é lock pessimista dentro de transação ou uma atualização atômica com a condição no próprio UPDATE:

DB::transaction(function () use ($id, $amountCents) {
    $account = Account::whereKey($id)->lockForUpdate()->firstOrFail();

    if ($account->balance < $amountCents) {
        throw new InsufficientFunds();
    }

    $account->decrement('balance', $amountCents);
});

O ponto de review não é só ver se existe lockForUpdate(). É perguntar se todos os caminhos que alteram aquele saldo usam o mesmo mecanismo. Um lock que só metade do código respeita não protege nada. Escrevi com mais calma sobre isso em como evitar race condition no saldo da conta digital com Laravel e PostgreSQL.

O que olhar na posição das transações, jobs e chamadas externas?

Esse é o bug que mais passa em review porque o código parece correto. Um job disparado dentro de DB::transaction() pode ser consumido pelo worker antes do commit e não encontrar o registro, ou pior, ser executado mesmo quando a transação faz rollback.

No diff, eu leio cada DB::transaction e marco o que acontece dentro dela:

O que está dentro da transação Risco O que pedir
dispatch() de job Job roda antes do commit ou após rollback ->afterCommit() ou after_commit na conexão da fila
Chamada HTTP ao gateway Lock segurado durante latência de rede; efeito externo sem rollback Mover para fora, com estado intermediário (ex.: processing)
Evento com listeners síncronos Listener faz I/O dentro do lock ShouldDispatchAfterCommit ou listener em fila
Envio de e-mail ou WhatsApp Cliente notificado de algo que não foi gravado Disparar após o commit 
DB::transaction(function () use ($payout) {
    $payout->update(['status' => 'queued']);
    ProcessPayout::dispatch($payout)->afterCommit();
});

O detalhe todo está em jobs disparados dentro de transação e o bug silencioso do afterCommit. Em review, basta lembrar a regra: dentro da transação só entra o que o banco consegue desfazer.

Que perguntas fazer sobre status, estornos e split?

Aqui o review é mais conversa do que leitura de código. Quando o PR adiciona ou altera um status de pagamento, eu pergunto:

  1. De quais status é possível chegar neste novo status? E sair dele?
  2. O que acontece se o webhook de paid chegar depois do de refunded?
  3. Esse update(['status' => ...]) passa pela regra de transição ou escreve direto?

Qualquer atribuição direta de status fora do lugar que centraliza as transições é bloqueante para mim.

Para estornos, a pergunta é sempre a mesma: e se for parcial, e se for o segundo estorno parcial? Código que só foi pensado para estorno total costuma calcular o valor restante errado ou devolver a taxa duas vezes. Em PRs de split, peço um teste em que o valor não divide exato entre os recebedores, por exemplo R$ 10,00 entre três partes, e confiro quem fica com o centavo que sobra. Se a resposta for "depende", a regra ainda não foi decidida, e isso é mais importante do que o código.

O que verificar em logs e dados sensíveis?

Integrações financeiras incentivam logar tudo, porque debugar sem payload é sofrido. O risco é o log virar um depósito de CPF, dados bancários, tokens de cartão e documentos de KYC vindos do Sumsub, guardados sem prazo e acessíveis a qualquer um com acesso ao Papertrail ou ao CloudWatch.

No diff, procuro:

  1. Log::info($request->all()) ou json_encode($response->json()) sem filtro.
  2. Payloads de webhook salvos em tabela sem política de retenção.
  3. Exceptions que incluem o corpo da resposta do provedor na mensagem, que depois vai parar no Sentry.
  4. Chaves de API ou segredos de webhook em mensagem de erro.

A correção costuma ser simples: uma lista de campos permitidos no log (allowlist) em vez de uma lista de campos removidos, e mascaramento de documento. Allowlist é melhor porque o provedor pode adicionar um campo sensível amanhã e você não vai ficar sabendo.

Como revisar os testes de um PR financeiro?

O teste que mais encontro em PR de pagamento é o caminho feliz com o gateway respondendo 200. Ele é necessário, mas quase nunca é onde o bug está. Eu peço pelo menos um teste de cada tipo abaixo quando o PR toca numa integração:

  1. Timeout seguido de sucesso, para provar que o retry não duplica.
  2. Evento duplicado, para provar a idempotência.
  3. Eventos fora de ordem, para provar a máquina de estados.
  4. Valores que não dividem exato, para provar o arredondamento.
Http::fake([
    'api.gateway.test/*' => Http::sequence()
        ->pushStatus(504)
        ->push(['id' => 'tr_123', 'status' => 'paid'], 200),
]);

$this->artisan('queue:work --once');
$this->artisan('queue:work --once');

expect(Charge::where('external_id', 'tr_123')->count())->toBe(1);
Http::assertSentCount(2);

Se o autor do PR não consegue escrever esse teste porque o código não permite simular a falha, isso já é um achado de review: o design está acoplado demais ao cliente HTTP.

Como fazer review rigoroso sem virar gargalo do time?

Esse é o lado de carreira da coisa. Ser o revisor que encontra tudo e devolve PR com quarenta comentários não escala, e o time aprende a fugir de você. O que funcionou melhor para mim:

  1. Classificar cada comentário por severidade. Uso três níveis: bloqueante (perda de dinheiro, dado vazado, inconsistência), importante (deve ser resolvido, mas pode virar issue) e sugestão (gosto pessoal, o autor decide). Isso deixa claro o que impede o merge.
  2. Transformar o checklist em template de PR. Quem abre o PR responde as perguntas antes de pedir review. Metade dos problemas o próprio autor encontra.
  3. Automatizar o que é mecânico. Larastan e testes de arquitetura do Pest pegam uso de float e funções proibidas em namespaces de domínio financeiro, liberando o review humano para o que exige raciocínio.
  4. Explicar o porquê uma vez, em um lugar só. Em vez de repetir em cada PR por que não se usa float, eu linko o documento interno. O comentário fica curto e o conhecimento fica no time.

Um exemplo do template que coloco em .github/pull_request_template.md:

## Este PR movimenta valores?
- [ ] Valores em centavos (int), arredondamento explícito
- [ ] Chamadas externas e jobs são idempotentes (qual a chave?)
- [ ] Nenhum job, evento ou HTTP dentro de transação sem afterCommit
- [ ] Mudanças de status passam pela regra de transição
- [ ] Testes de timeout, duplicidade e ordem trocada
- [ ] Logs sem CPF, dados bancários ou tokens

E o teste de arquitetura, que roda no CI:

arch('domínio financeiro não usa float')
    ->expect('App\\Domain\\Payments')
    ->not->toUse(['floatval', 'round', 'number_format']);

Para quem trabalha sozinho, como eu em boa parte dos produtos, o checklist vira self-review. A regra que adotei é não revisar o próprio PR financeiro no mesmo momento em que terminei de escrever. Abro o diff no dia seguinte, com o checklist ao lado, e leio como se fosse código de outra pessoa. É impressionante quanta coisa aparece com algumas horas de distância.

O que fazer a partir de amanhã

Se você revisa ou escreve código que mexe com dinheiro, comece pequeno: copie as seis perguntas do template para o repositório e exija que todo PR financeiro responda a elas. Na primeira semana, observe quais itens mais aparecem marcados como problema no seu time. Esse é o lugar onde vale investir em automação, seja um teste de arquitetura, uma regra do Larastan ou uma abstração de dinheiro que torne o erro impossível de escrever.

O revisor sênior em fintech não é quem lê mais rápido. É quem sempre pergunta o que acontece quando o código roda duas vezes, ao mesmo tempo ou pela metade, e que ensina o time a fazer essa pergunta antes de abrir o PR.