mirror of
https://github.com/bisq-network/bisq.git
synced 2025-02-23 15:00:30 +01:00
Cleanup TODOs
This commit is contained in:
Manfred Karrer
parent
7e61afcb8e
commit
92fdca2abd
16 changed files with 39 additions and 90 deletions
|
|
@ -59,13 +59,11 @@ public class BlindVoteValidator {
|
|||
checkArgument(blindVote.getEncryptedVotes().length > 0,
|
||||
"encryptedProposalList must not be empty");
|
||||
checkNotNull(blindVote.getTxId(), "txId must not be null");
|
||||
checkArgument(blindVote.getTxId().length() > 0, "txId must not be empty");
|
||||
checkArgument(!blindVote.getTxId().isEmpty(), "txId must not be empty");
|
||||
checkArgument(blindVote.getStake() > 0, "stake must be positive");
|
||||
checkNotNull(blindVote.getEncryptedMeritList(), "getEncryptedMeritList must not be null");
|
||||
checkArgument(blindVote.getEncryptedMeritList().length > 0,
|
||||
"getEncryptedMeritList must not be empty");
|
||||
|
||||
//TODO should we use a min/max for stake, atm its just dust limit as the min. value
|
||||
} catch (Throwable e) {
|
||||
log.warn(e.toString());
|
||||
throw new ProposalValidationException(e);
|
||||
|
|
|
|||
|
|
@ -339,8 +339,6 @@ public class MyBlindVoteListService implements PersistedDataHost, DaoStateListen
|
|||
|
||||
@Override
|
||||
public void onFailure(TxBroadcastException exception) {
|
||||
// TODO handle
|
||||
// We need to be sure that in case of a failed tx the locked stake gets unlocked!
|
||||
exceptionHandler.handleException(exception);
|
||||
}
|
||||
});
|
||||
|
|
|
|||
|
|
@ -50,7 +50,6 @@ import lombok.extern.slf4j.Slf4j;
|
|||
* unconfirmed txs.
|
||||
*/
|
||||
@Slf4j
|
||||
//TODO maybe extend BondRepository as well?
|
||||
public class MyBondedReputationRepository implements DaoSetupService {
|
||||
private final DaoStateService daoStateService;
|
||||
private final BsqWalletService bsqWalletService;
|
||||
|
|
|
|||
|
|
@ -94,10 +94,6 @@ public class MeritConsensus {
|
|||
return false;
|
||||
}
|
||||
|
||||
// TODO Check if a sig key was used multiple times for different voters
|
||||
// At the moment we don't impl. that to not add too much complexity and as we consider that
|
||||
// risk very low.
|
||||
|
||||
boolean result = false;
|
||||
try {
|
||||
ECKey pubKey = ECKey.fromPublicOnly(Utilities.decodeFromHex(pubKeyAsHex));
|
||||
|
|
|
|||
|
|
@ -141,7 +141,7 @@ public abstract class ProposalValidator implements ConsensusCritical {
|
|||
}
|
||||
}
|
||||
|
||||
protected Integer getBlockHeight(Proposal proposal) {
|
||||
protected int getBlockHeight(Proposal proposal) {
|
||||
// When we receive a temp proposal the tx is usually not confirmed so we cannot lookup the block height of
|
||||
// the tx. We take the current block height in that case as it would be in the same cycle anyway.
|
||||
return daoStateService.getTx(proposal.getTxId())
|
||||
|
|
|
|||
|
|
@ -42,7 +42,7 @@ import lombok.extern.slf4j.Slf4j;
|
|||
|
||||
import static com.google.common.base.Preconditions.checkArgument;
|
||||
|
||||
//TODO Use translation properties in error messages a they are shown to user.
|
||||
//TODO Use translation properties in error messages a they are shown to user. show min/max values in errors
|
||||
|
||||
/**
|
||||
* Changes here can potentially break consensus!
|
||||
|
|
@ -292,7 +292,7 @@ public class ChangeParamValidator extends ProposalValidator implements Consensus
|
|||
if (max == 0)
|
||||
return Result.OK;
|
||||
|
||||
//TODO some cases with min = 0 and max not 0 or the other way round are not correclty implemented yet.
|
||||
//TODO some cases with min = 0 and max not 0 or the other way round are not correctly implemented yet.
|
||||
// Not intended to be used that way anyway but should be fixed...
|
||||
double change = currentValue != 0 ? newValue / currentValue : 0;
|
||||
if (change > max)
|
||||
|
|
|
|||
|
|
@ -213,15 +213,13 @@ public class VoteResultService implements DaoStateListener, DaoSetupService {
|
|||
// it to our state. Otherwise we are not in consensus with the network.
|
||||
daoStateService.addDecryptedBallotsWithMeritsSet(filteredDecryptedBallotsWithMeritsSet);
|
||||
|
||||
// FIXME we got duplicated items in evaluatedProposals with diff. merit values, find out why...
|
||||
Set<EvaluatedProposal> evaluatedProposals = getEvaluatedProposals(filteredDecryptedBallotsWithMeritsSet, chainHeight);
|
||||
daoStateService.addEvaluatedProposalSet(evaluatedProposals);
|
||||
Set<EvaluatedProposal> acceptedEvaluatedProposals = getAcceptedEvaluatedProposals(evaluatedProposals);
|
||||
applyAcceptedProposals(acceptedEvaluatedProposals, chainHeight);
|
||||
log.info("processAllVoteResults completed");
|
||||
} else {
|
||||
// TODO make sure we handle it in the UI -> ask user to restart
|
||||
String msg = "We could not find a list which matches the majority so we cannot calculate the vote result.";
|
||||
String msg = "We could not find a list which matches the majority so we cannot calculate the vote result. Please restart and resync the DAO state.";
|
||||
log.warn(msg);
|
||||
voteResultExceptions.add(new VoteResultException(currentCycle, new Exception(msg)));
|
||||
}
|
||||
|
|
@ -236,8 +234,7 @@ public class VoteResultService implements DaoStateListener, DaoSetupService {
|
|||
}
|
||||
|
||||
// Those which did not get accepted will be added to the nonBsq map
|
||||
// FIXME add check for cycle as now we call addNonBsqTxOutput for past rejected comp requests as well
|
||||
daoStateService.getIssuanceCandidateTxOutputs().stream()
|
||||
daoStateService.getIssuanceCandidateTxOutputsOfCurrentCycle().stream()
|
||||
.filter(txOutput -> !daoStateService.isIssuanceTx(txOutput.getTxId()))
|
||||
.forEach(daoStateService::addNonBsqTxOutput);
|
||||
|
||||
|
|
@ -276,8 +273,6 @@ public class VoteResultService implements DaoStateListener, DaoSetupService {
|
|||
checkArgument(optionalVoteRevealTx.isPresent(), "optionalVoteRevealTx must be present. voteRevealTxId=" + voteRevealTxId);
|
||||
Tx voteRevealTx = optionalVoteRevealTx.get();
|
||||
|
||||
// TODO maybe verify version in opReturn
|
||||
|
||||
// Here we use only blockchain tx data so far so we don't have risks with missing P2P network data.
|
||||
// We work back from the voteRealTx to the blindVoteTx to caclulate the majority hash. From that we
|
||||
// will derive the blind vote list we will use for result calculation and as it was based on
|
||||
|
|
@ -454,8 +449,6 @@ public class VoteResultService implements DaoStateListener, DaoSetupService {
|
|||
// It still could be that we have additional blind votes so our hash does not match. We can try to permute
|
||||
// our list with excluding items to see if we get a matching list. If not last resort is to request the
|
||||
// missing items from the network.
|
||||
// TODO we should add some metadata about the published blind vote txId list so it becomes easier to find
|
||||
// the majority list. We could add a new payload for that or add a message to request the list from peers.
|
||||
Optional<List<BlindVote>> permutatedList = findPermutatedListMatchingMajority(majorityVoteListHash);
|
||||
if (permutatedList.isPresent()) {
|
||||
return permutatedList;
|
||||
|
|
@ -503,12 +496,10 @@ public class VoteResultService implements DaoStateListener, DaoSetupService {
|
|||
// We reorganize the data structure to have a map of proposals with a list of VoteWithStake objects
|
||||
Map<Proposal, List<VoteWithStake>> resultListByProposalMap = getVoteWithStakeListByProposalMap(decryptedBallotsWithMeritsSet);
|
||||
|
||||
// TODO breakup
|
||||
Set<EvaluatedProposal> evaluatedProposals = new HashSet<>();
|
||||
resultListByProposalMap.forEach((proposal, voteWithStakeList) -> {
|
||||
long requiredQuorum = daoStateService.getParamValueAsCoin(proposal.getQuorumParam(), chainHeight).value;
|
||||
long requiredVoteThreshold = getRequiredVoteThreshold(chainHeight, proposal);
|
||||
//TODO add checks for param change that input for quorum param of <5000 is not allowed
|
||||
checkArgument(requiredVoteThreshold >= 5000,
|
||||
"requiredVoteThreshold must be not be less then 50% otherwise we could have conflicting results.");
|
||||
|
||||
|
|
@ -652,24 +643,6 @@ public class VoteResultService implements DaoStateListener, DaoSetupService {
|
|||
log.warn("There have been multiple winning param change proposals with the same item. " +
|
||||
"This is a sign of a social consensus failure. " +
|
||||
"We treat all requests as failed in such a case.");
|
||||
|
||||
// TODO remove code once we are 100% sure we stick with the above solution.
|
||||
// We got multiple proposals for the same parameter. We check which one got the higher stake and that
|
||||
// one will be the winner. If both have same stake none will be the winner.
|
||||
/*list.sort(Comparator.comparing(ev -> ev.getProposalVoteResult().getStakeOfAcceptedVotes()));
|
||||
Collections.reverse(list);
|
||||
EvaluatedProposal first = list.get(0);
|
||||
EvaluatedProposal second = list.get(1);
|
||||
if (first.getProposalVoteResult().getStakeOfAcceptedVotes() >
|
||||
second.getProposalVoteResult().getStakeOfAcceptedVotes()) {
|
||||
applyAcceptedChangeParamProposal((ChangeParamProposal) first.getProposal(), chainHeight);
|
||||
} else {
|
||||
// Rare case that both have the same stake. We don't need to check for a third entry as if 2 have
|
||||
// the same we are already in the abort case to reject all proposals with that param
|
||||
log.warn("We got the rare case that multiple changeParamProposals have received the same stake. " +
|
||||
"None will be accepted in such a case.\n" +
|
||||
"EvaluatedProposal={}", list);
|
||||
}*/
|
||||
}
|
||||
});
|
||||
}
|
||||
|
|
|
|||
|
|
@ -37,7 +37,6 @@ import lombok.extern.slf4j.Slf4j;
|
|||
|
||||
import static com.google.common.base.Preconditions.checkArgument;
|
||||
|
||||
//TODO case that user misses reveal phase not impl. yet
|
||||
|
||||
@Slf4j
|
||||
public class IssuanceService {
|
||||
|
|
@ -72,27 +71,23 @@ public class IssuanceService {
|
|||
// bsqWallet would show that spent state). We would need to support a spent status for the outputs
|
||||
// which are interpreted as BTC (as a not yet accepted comp. request).
|
||||
Optional<Tx> optionalTx = daoStateService.getTx(issuanceProposal.getTxId());
|
||||
if (optionalTx.isPresent()) {
|
||||
long amount = issuanceProposal.getRequestedBsq().value;
|
||||
Tx tx = optionalTx.get();
|
||||
// We use key from first input
|
||||
TxInput txInput = tx.getTxInputs().get(0);
|
||||
String pubKey = txInput.getPubKey();
|
||||
Issuance issuance = new Issuance(tx.getId(), chainHeight, amount, pubKey, issuanceType);
|
||||
daoStateService.addIssuance(issuance);
|
||||
daoStateService.addUnspentTxOutput(txOutput);
|
||||
checkArgument(optionalTx.isPresent(), "optionalTx must be present");
|
||||
long amount = issuanceProposal.getRequestedBsq().value;
|
||||
Tx tx = optionalTx.get();
|
||||
// We use key from first input
|
||||
TxInput txInput = tx.getTxInputs().get(0);
|
||||
String pubKey = txInput.getPubKey();
|
||||
Issuance issuance = new Issuance(tx.getId(), chainHeight, amount, pubKey, issuanceType);
|
||||
daoStateService.addIssuance(issuance);
|
||||
daoStateService.addUnspentTxOutput(txOutput);
|
||||
|
||||
StringBuilder sb = new StringBuilder();
|
||||
sb.append("\n################################################################################\n");
|
||||
sb.append("We issued new BSQ to tx with ID ").append(txOutput.getTxId())
|
||||
.append("\nIssued BSQ: ").append(issuanceProposal.getRequestedBsq())
|
||||
.append("\nIssuance type: ").append(issuanceType.name())
|
||||
.append("\n################################################################################\n");
|
||||
log.info(sb.toString());
|
||||
} else {
|
||||
//TODO throw exception
|
||||
log.error("Tx for compensation request not found. txId={}", issuanceProposal.getTxId());
|
||||
}
|
||||
StringBuilder sb = new StringBuilder();
|
||||
sb.append("\n################################################################################\n");
|
||||
sb.append("We issued new BSQ to tx with ID ").append(txOutput.getTxId())
|
||||
.append("\nIssued BSQ: ").append(issuanceProposal.getRequestedBsq())
|
||||
.append("\nIssuance type: ").append(issuanceType.name())
|
||||
.append("\n################################################################################\n");
|
||||
log.info(sb.toString());
|
||||
});
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -32,7 +32,6 @@ import bisq.core.dao.governance.blindvote.storage.BlindVotePayload;
|
|||
import bisq.core.dao.governance.myvote.MyVote;
|
||||
import bisq.core.dao.governance.myvote.MyVoteListService;
|
||||
import bisq.core.dao.governance.period.PeriodService;
|
||||
import bisq.core.dao.node.BsqNode;
|
||||
import bisq.core.dao.node.BsqNodeProvider;
|
||||
import bisq.core.dao.state.DaoStateListener;
|
||||
import bisq.core.dao.state.DaoStateService;
|
||||
|
|
@ -62,10 +61,7 @@ import java.util.List;
|
|||
import lombok.Getter;
|
||||
import lombok.extern.slf4j.Slf4j;
|
||||
|
||||
//TODO case that user misses reveal phase not impl. yet
|
||||
|
||||
|
||||
// TODO We could also broadcast the winning list at the moment the reveal period is over and have the break
|
||||
// TODO Broadcast the winning list at the moment the reveal period is over and have the break
|
||||
// interval as time buffer for all nodes to receive that winning list. All nodes which are in sync with the
|
||||
// majority data view can broadcast. That way it will become a very unlikely case that a node is missing
|
||||
// data.
|
||||
|
|
@ -90,10 +86,8 @@ public class VoteRevealService implements DaoStateListener, DaoSetupService {
|
|||
private final P2PService p2PService;
|
||||
private final WalletsManager walletsManager;
|
||||
|
||||
//TODO UI should listen to that
|
||||
@Getter
|
||||
private final ObservableList<VoteRevealException> voteRevealExceptions = FXCollections.observableArrayList();
|
||||
private final BsqNode bsqNode;
|
||||
private final List<VoteRevealTxPublishedListener> voteRevealTxPublishedListeners = new ArrayList<>();
|
||||
|
||||
///////////////////////////////////////////////////////////////////////////////////////////
|
||||
|
|
@ -118,8 +112,6 @@ public class VoteRevealService implements DaoStateListener, DaoSetupService {
|
|||
this.btcWalletService = btcWalletService;
|
||||
this.p2PService = p2PService;
|
||||
this.walletsManager = walletsManager;
|
||||
|
||||
bsqNode = bsqNodeProvider.getBsqNode();
|
||||
}
|
||||
|
||||
|
||||
|
|
@ -224,7 +216,7 @@ public class VoteRevealService implements DaoStateListener, DaoSetupService {
|
|||
});
|
||||
}
|
||||
|
||||
private void revealVote(MyVote myVote, boolean inBlindVotePhase) {
|
||||
private void revealVote(MyVote myVote, boolean isInVoteRevealPhase) {
|
||||
try {
|
||||
// We collect all valid blind vote items we received via the p2p network.
|
||||
// It might be that different nodes have a different collection of those items.
|
||||
|
|
@ -234,7 +226,7 @@ public class VoteRevealService implements DaoStateListener, DaoSetupService {
|
|||
|
||||
// If we are not in the right phase we just add an empty hash (still need to have the hash as otherwise we
|
||||
// would not recognize the tx as vote reveal tx)
|
||||
byte[] hashOfBlindVoteList = inBlindVotePhase ? getHashOfBlindVoteList() : new byte[20];
|
||||
byte[] hashOfBlindVoteList = isInVoteRevealPhase ? getHashOfBlindVoteList() : new byte[20];
|
||||
byte[] opReturnData = VoteRevealConsensus.getOpReturnData(hashOfBlindVoteList, myVote.getSecretKey());
|
||||
|
||||
// We search for my unspent stake output.
|
||||
|
|
@ -245,8 +237,6 @@ public class VoteRevealService implements DaoStateListener, DaoSetupService {
|
|||
.findFirst()
|
||||
.orElseThrow(() -> new VoteRevealException("stakeTxOutput is not found for myVote.", myVote));
|
||||
|
||||
// TxOutput has to be in the current cycle. Phase is checked in the parser anyway.
|
||||
// TODO is phase check needed and done in parser still?
|
||||
Transaction voteRevealTx = getVoteRevealTx(stakeTxOutput, opReturnData);
|
||||
log.info("voteRevealTx={}", voteRevealTx);
|
||||
publishTx(voteRevealTx);
|
||||
|
|
@ -255,7 +245,7 @@ public class VoteRevealService implements DaoStateListener, DaoSetupService {
|
|||
// next startup but the tx was actually broadcasted.
|
||||
myVoteListService.applyRevealTxId(myVote, voteRevealTx.getHashAsString());
|
||||
|
||||
if (inBlindVotePhase) {
|
||||
if (isInVoteRevealPhase) {
|
||||
// Just for additional resilience we republish our blind votes
|
||||
List<BlindVote> sortedBlindVoteListOfCycle = BlindVoteConsensus.getSortedBlindVoteListOfCycle(blindVoteListService);
|
||||
rePublishBlindVotePayloadList(sortedBlindVoteListOfCycle);
|
||||
|
|
@ -280,7 +270,6 @@ public class VoteRevealService implements DaoStateListener, DaoSetupService {
|
|||
@Override
|
||||
public void onFailure(TxBroadcastException exception) {
|
||||
log.error(exception.toString());
|
||||
// TODO handle
|
||||
voteRevealExceptions.add(new VoteRevealException("Publishing of voteRevealTx failed.",
|
||||
exception, voteRevealTx));
|
||||
}
|
||||
|
|
|
|||
|
|
@ -32,8 +32,6 @@ import io.bisq.generated.protobuffer.PB;
|
|||
import lombok.EqualsAndHashCode;
|
||||
import lombok.Getter;
|
||||
|
||||
// TODO CapabilityRequiringPayload does only cover add data messages. We need a tool to avoid disconnections to old
|
||||
// nodes when they receive the NewBlockBroadcastMessage!
|
||||
@EqualsAndHashCode(callSuper = true)
|
||||
@Getter
|
||||
public final class NewBlockBroadcastMessage extends BroadcastMessage implements CapabilityRequiringPayload {
|
||||
|
|
|
|||
|
|
@ -93,7 +93,6 @@ public class BlockParser {
|
|||
rawBlock.getPreviousBlockHash());
|
||||
|
||||
if (isBlockAlreadyAdded(rawBlock)) {
|
||||
//TODO check how/if that can happen
|
||||
log.warn("Block was already added.");
|
||||
DevEnv.logErrorAndThrowIfDevMode("Block was already added. rawBlock=" + rawBlock);
|
||||
} else {
|
||||
|
|
|
|||
|
|
@ -181,7 +181,6 @@ public class TxParser {
|
|||
* It verifies also if the fee is correct (if required) and if the phase is correct (if relevant).
|
||||
* We set the txType as well as the txOutputType of the relevant outputs.
|
||||
*/
|
||||
// TODO That method is not testable and still too complex.
|
||||
private void applyTxTypeAndTxOutputType(int blockHeight, TempTx tempTx, long bsqFee) {
|
||||
OpReturnType opReturnType = null;
|
||||
Optional<OpReturnType> optionalOpReturnType = txOutputParser.getOptionalOpReturnType();
|
||||
|
|
|
|||
|
|
@ -277,8 +277,6 @@ public class DaoStateService implements DaoSetupService {
|
|||
* {@code false}.
|
||||
*/
|
||||
public boolean isBlockHashKnown(String blockHash) {
|
||||
// TODO(chirhonul): If performance of O(n) time in number of blocks becomes an issue,
|
||||
// we should keep a HashMap of block hash -> Block to make this method O(1).
|
||||
return getBlocks().stream().anyMatch(block -> block.getHash().equals(blockHash));
|
||||
}
|
||||
|
||||
|
|
@ -570,6 +568,16 @@ public class DaoStateService implements DaoSetupService {
|
|||
return getTxOutputsByTxOutputType(TxOutputType.ISSUANCE_CANDIDATE_OUTPUT);
|
||||
}
|
||||
|
||||
public Set<TxOutput> getIssuanceCandidateTxOutputsOfCurrentCycle() {
|
||||
Cycle currentCycle = getCurrentCycle();
|
||||
if (currentCycle == null)
|
||||
return new HashSet<>();
|
||||
|
||||
return getIssuanceCandidateTxOutputs().stream()
|
||||
.filter(txOutput -> currentCycle.isInCycle(txOutput.getBlockHeight()))
|
||||
.collect(Collectors.toSet());
|
||||
}
|
||||
|
||||
|
||||
///////////////////////////////////////////////////////////////////////////////////////////
|
||||
// Issuance
|
||||
|
|
|
|||
|
|
@ -136,7 +136,6 @@ public final class Tx extends BaseTx implements PersistablePayload, ImmutableDao
|
|||
|
||||
|
||||
public int getLockTime() {
|
||||
// TODO MK: Still get confused that we have the lock time stored at a non opReturn output
|
||||
return getLockupOutput().getLockTime();
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -43,9 +43,10 @@ public class Issuance implements PersistablePayload, NetworkPayload, ImmutableDa
|
|||
private final int chainHeight; // of issuance (first block of result phase)
|
||||
private final long amount;
|
||||
|
||||
//TODO do we need to store that? its in the blockchain anyway
|
||||
// sig key as hex of first input in issuance tx used for signing the merits
|
||||
// Can be null (payToPubKey tx) but in our case it will never be null. Still keep it nullable to be safe.
|
||||
@Nullable
|
||||
private final String pubKey; // sig key as hex of first input in issuance tx
|
||||
private final String pubKey;
|
||||
|
||||
private final IssuanceType issuanceType;
|
||||
|
||||
|
|
@ -68,9 +69,7 @@ public class Issuance implements PersistablePayload, NetworkPayload, ImmutableDa
|
|||
.setChainHeight(chainHeight)
|
||||
.setAmount(amount)
|
||||
.setIssuanceType(issuanceType.name());
|
||||
|
||||
Optional.ofNullable(pubKey).ifPresent(e -> builder.setPubKey(pubKey));
|
||||
|
||||
return builder.build();
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -145,7 +145,6 @@ public class UnconfirmedBsqChangeOutputListService implements PersistedDataHost
|
|||
persist();
|
||||
}
|
||||
|
||||
//TODO not sure how we should handle it but seems safest to just clear all
|
||||
public void onReorganize() {
|
||||
unconfirmedBsqChangeOutputList.clear();
|
||||
persist();
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue