Skip to content

Commit

Permalink
Add signing debug logs (#3948)
Browse files Browse the repository at this point in the history
* Add signing debug logs

Fix warning about possible nullpointer exceptions. Better to have an
explicit check for debug purposes.

Smaller cleanups

* Fix codacy comment
  • Loading branch information
sqrrm authored Feb 7, 2020
1 parent d3f30b8 commit ac7d636
Show file tree
Hide file tree
Showing 7 changed files with 81 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@

import lombok.extern.slf4j.Slf4j;

import javax.annotation.Nullable;

import static com.google.common.base.Preconditions.checkNotNull;

@Slf4j
Expand Down Expand Up @@ -736,4 +738,64 @@ public SignState getSignState(AccountAgeWitness accountAgeWitness) {
}
}
}

///////////////////////////////////////////////////////////////////////////////////////////
// Debug logs
///////////////////////////////////////////////////////////////////////////////////////////
private String getWitnessDebugLog(PaymentAccountPayload paymentAccountPayload,
PubKeyRing pubKeyRing) {
Optional<AccountAgeWitness> accountAgeWitness = findWitness(paymentAccountPayload, pubKeyRing);
if (!accountAgeWitness.isPresent()) {
byte[] accountInputDataWithSalt = getAccountInputDataWithSalt(paymentAccountPayload);
byte[] hash = Hash.getSha256Ripemd160hash(Utilities.concatenateByteArrays(accountInputDataWithSalt,
pubKeyRing.getSignaturePubKeyBytes()));
return "No accountAgeWitness found for paymentAccountPayload with hash " + Utilities.bytesAsHexString(hash);
}

SignState signState = getSignState(accountAgeWitness.get());
return signState.name() + " " + signState.getPresentation() +
"\n" + accountAgeWitness.toString();
}

public void witnessDebugLog(Trade trade, @Nullable AccountAgeWitness myWitness) {
// Log to find why accounts sometimes don't get signed as expected
// TODO: Demote to debug or remove once account signing is working ok
checkNotNull(trade.getContract());
checkNotNull(trade.getContract().getBuyerPaymentAccountPayload());
boolean checkingSignTrade = true;
boolean isBuyer = trade.getContract().isMyRoleBuyer(keyRing.getPubKeyRing());
AccountAgeWitness witness = myWitness;
if (witness == null) {
witness = isBuyer ?
getMyWitness(trade.getContract().getBuyerPaymentAccountPayload()) :
getMyWitness(trade.getContract().getSellerPaymentAccountPayload());
checkingSignTrade = false;
}
boolean isSignWitnessTrade = accountIsSigner(witness) &&
!peerHasSignedWitness(trade) &&
tradeAmountIsSufficient(trade.getTradeAmount());
log.info("AccountSigning: " +
"\ntradeId: {}" +
"\nis buyer: {}" +
"\nbuyer account age witness info: {}" +
"\nseller account age witness info: {}" +
"\nchecking for sign trade: {}" +
"\nis myWitness signer: {}" +
"\npeer has signed witness: {}" +
"\ntrade amount: {}" +
"\ntrade amount is sufficient: {}" +
"\nisSignWitnessTrade: {}",
trade.getId(),
isBuyer,
getWitnessDebugLog(trade.getContract().getBuyerPaymentAccountPayload(),
trade.getContract().getBuyerPubKeyRing()),
getWitnessDebugLog(trade.getContract().getSellerPaymentAccountPayload(),
trade.getContract().getSellerPubKeyRing()),
checkingSignTrade, // Following cases added to use same logic as in seller signing check
accountIsSigner(witness),
peerHasSignedWitness(trade),
trade.getTradeAmount(),
tradeAmountIsSufficient(trade.getTradeAmount()),
isSignWitnessTrade);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@ private void handle(DepositTxAndDelayedPayoutTxMessage tradeMessage, NodeAddress
PublishTradeStatistics.class
);
taskRunner.run();
processModel.logTrade(buyerAsMakerTrade);
}

private void handle(PayoutTxPublishedMessage tradeMessage, NodeAddress peerNodeAddress) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,7 @@ private void handle(DepositTxAndDelayedPayoutTxMessage tradeMessage, NodeAddress
PublishTradeStatistics.class
);
taskRunner.run();
processModel.logTrade(buyerAsTakerTrade);
}

private void handle(PayoutTxPublishedMessage tradeMessage, NodeAddress peerNodeAddress) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ public NodeAddress getMyNodeAddress() {
return p2PService.getAddress();
}

public void setPaymentStartedAckMessage(AckMessage ackMessage) {
void setPaymentStartedAckMessage(AckMessage ackMessage) {
if (ackMessage.isSuccess()) {
setPaymentStartedMessageState(MessageState.ACKNOWLEDGED);
} else {
Expand All @@ -356,4 +356,8 @@ private void setAccountId(String accountId) {
private void setPubKeyRing(PubKeyRing pubKeyRing) {
this.pubKeyRing = pubKeyRing;
}

void logTrade(Trade trade) {
accountAgeWitnessService.witnessDebugLog(trade, null);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -140,9 +140,7 @@ protected void handle(DepositTxMessage tradeMessage, NodeAddress sender) {
processModel.setTempTradingPeerNodeAddress(sender);

TradeTaskRunner taskRunner = new TradeTaskRunner(sellerAsMakerTrade,
() -> {
handleTaskRunnerSuccess(tradeMessage, "DepositTxPublishedMessage");
},
() -> handleTaskRunnerSuccess(tradeMessage, "DepositTxPublishedMessage"),
errorMessage -> handleTaskRunnerFault(tradeMessage, errorMessage));

taskRunner.addTasks(
Expand Down Expand Up @@ -174,6 +172,7 @@ private void handle(DelayedPayoutTxSignatureResponse tradeMessage, NodeAddress s
PublishTradeStatistics.class
);
taskRunner.run();
processModel.logTrade(sellerAsMakerTrade);
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ private void handle(DelayedPayoutTxSignatureResponse tradeMessage, NodeAddress s
PublishTradeStatistics.class
);
taskRunner.run();
processModel.logTrade(sellerAsTakerTrade);
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,8 @@ String getMarketLabel(PendingTradesListItem item) {
if ((item == null))
return "";

checkNotNull(item.getTrade().getOffer());
checkNotNull(item.getTrade().getOffer().getCurrencyCode());
return CurrencyUtil.getCurrencyPair(item.getTrade().getOffer().getCurrencyCode());
}

Expand Down Expand Up @@ -260,6 +262,8 @@ String getMyRole(PendingTradesListItem item) {
Contract contract = trade.getContract();
if (contract != null) {
Offer offer = trade.getOffer();
checkNotNull(offer);
checkNotNull(offer.getCurrencyCode());
return getRole(contract.isBuyerMakerAndSellerTaker(), dataModel.isMaker(offer), offer.getCurrencyCode());
} else {
return "";
Expand All @@ -270,6 +274,8 @@ String getPaymentMethod(PendingTradesListItem item) {
String result = "";
if (item != null) {
Offer offer = item.getTrade().getOffer();
checkNotNull(offer);
checkNotNull(offer.getPaymentMethod());
String method = Res.get(offer.getPaymentMethod().getId() + "_SHORT");
String methodCountryCode = offer.getCountryCode();

Expand Down Expand Up @@ -303,6 +309,7 @@ public String getTxFee() {

public String getTradeFee() {
if (trade != null && dataModel.getOffer() != null && trade.getTradeAmount() != null) {
checkNotNull(dataModel.getTrade());
if (dataModel.isMaker() && dataModel.getOffer().isCurrencyForMakerFeeBtc() ||
!dataModel.isMaker() && dataModel.getTrade().isCurrencyForTakerFeeBtc()) {
Coin tradeFeeInBTC = dataModel.getTradeFeeInBTC();
Expand Down Expand Up @@ -373,6 +380,8 @@ public boolean isSignWitnessTrade() {
checkNotNull(trade.getOffer(), "offer must not be null");
AccountAgeWitness myWitness = accountAgeWitnessService.getMyWitness(dataModel.getSellersPaymentAccountPayload());

accountAgeWitnessService.witnessDebugLog(trade, myWitness);

return accountAgeWitnessService.accountIsSigner(myWitness) &&
!accountAgeWitnessService.peerHasSignedWitness(trade) &&
accountAgeWitnessService.tradeAmountIsSufficient(trade.getTradeAmount());
Expand Down

0 comments on commit ac7d636

Please sign in to comment.