Commit 1715993e authored by Luke Champine's avatar Luke Champine Committed by GitHub

Merge pull request #1580 from NebulousLabs/deadlocks

Fix AcceptTransactionSet Deadlocks
parents ee3d4988 ad9dc60a
......@@ -614,7 +614,7 @@ func TestRenterRelativePathErrorDownload(t *testing.T) {
renterDownloadAbsoluteError := "destination must be an absolute path"
// Create a file.
// Create a file, and upload it.
path := filepath.Join(st.dir, "test.dat")
if err = createRandFile(path, 1024); err != nil {
t.Fatal(err)
......@@ -624,26 +624,38 @@ func TestRenterRelativePathErrorDownload(t *testing.T) {
if err = st.stdPostAPI("/renter/upload/test", uploadValues); err != nil {
t.Fatal(err)
}
var rf RenterFiles
for i := 0; i < 100 && (len(rf.Files) != 1 || rf.Files[0].UploadProgress < 10); i++ {
st.getAPI("/renter/files", &rf)
time.Sleep(200 * time.Millisecond)
}
if len(rf.Files) != 1 || rf.Files[0].UploadProgress < 10 {
t.Fatal("the uploading is not succeeding for some reason:", rf.Files[0], rf.Files[1])
}
// This should fail.
// Use a relative destination, which should fail.
downloadPath := "test1.dat"
if err = st.stdGetAPI("/renter/download/test?destination=" + downloadPath); err.Error() != renterDownloadAbsoluteError {
t.Fatal(err)
}
// This should fail.
// Relative destination stepping backwards should also fail.
downloadPath = "../test1.dat"
if err = st.stdGetAPI("/renter/download/test?destination=" + downloadPath); err.Error() != renterDownloadAbsoluteError {
t.Fatal(err)
}
// This should fail.
//
// TODO: NDF failures here. What is supposed to be happening?
downloadPath = filepath.Join(st.dir, "test1.dat")
// Long relative destination should also fail (just missing leading slash).
downloadPath = filepath.Join(st.dir[1:], "test1.dat")
err = st.stdGetAPI("/renter/download/test?destination=" + downloadPath)
if err == nil {
t.Log(downloadPath)
t.Fatal("expecting an error")
}
// Full destination should succeed.
downloadPath = filepath.Join(st.dir, "test1.dat")
err = st.stdGetAPI("/renter/download/test?destination=" + downloadPath)
if err != nil {
t.Fatal("expecting an error")
}
}
......
......@@ -255,9 +255,7 @@ func (h *Host) managedFinalizeContract(builder modules.TransactionBuilder, rente
// just when the actual modification is happening.
i := 0
for {
h.mu.Lock()
err = h.addStorageObligation(so)
h.mu.Unlock()
err = h.managedAddStorageObligation(so)
if err == nil {
return nil
}
......
......@@ -296,85 +296,99 @@ func (h *Host) queueActionItem(height types.BlockHeight, id types.FileContractID
})
}
// addStorageObligation adds a storage obligation to the host. Because this
// operation can return errors, the transactions should not be submitted to the
// blockchain until after this function has indicated success. All of the
// sectors that are present in the storage obligation should already be on
// disk, which means that addStorageObligation should be exclusively called
// when creating a new, empty file contract or when renewing an existing file
// managedAddStorageObligation adds a storage obligation to the host. Because
// this operation can return errors, the transactions should not be submitted to
// the blockchain until after this function has indicated success. All of the
// sectors that are present in the storage obligation should already be on disk,
// which means that addStorageObligation should be exclusively called when
// creating a new, empty file contract or when renewing an existing file
// contract.
func (h *Host) addStorageObligation(so storageObligation) error {
// Sanity check - obligation should be under lock while being added.
soid := so.id()
_, exists := h.lockedStorageObligations[soid]
if !exists {
h.log.Critical("addStorageObligation called with an obligation that is not locked")
}
// Sanity check - There needs to be enough time left on the file contract
// for the host to safely submit the file contract revision.
if h.blockHeight+revisionSubmissionBuffer >= so.expiration() {
h.log.Critical("submission window was not verified before trying to submit a storage obligation")
return errNoBuffer
}
// Sanity check - the resubmission timeout needs to be smaller than storage
// proof window.
if so.expiration()+resubmissionTimeout >= so.proofDeadline() {
h.log.Critical("host is misconfigured - the storage proof window needs to be long enough to resubmit if needed")
return errors.New("fill me in")
}
// Add the storage obligation information to the database.
err := h.db.Update(func(tx *bolt.Tx) error {
// Sanity check - a storage obligation using the same file contract id
// should not already exist. This situation can happen if the
// transaction pool ejects a file contract and then a new one is
// created. Though the file contract will have the same terms, some
// other conditions might cause problems. The check for duplicate file
// contract ids should happen during the negotiation phase, and not
// during the 'addStorageObligation' phase.
bso := tx.Bucket(bucketStorageObligations)
// If the storage obligation already has sectors, it means that the
// file contract is being renewed, and that the sector should be
// re-added with a new expriation height. If there is an error at any
// point, all of the sectors should be removed.
if len(so.SectorRoots) != 0 {
err := h.AddSectorBatch(so.SectorRoots, so.expiration())
func (h *Host) managedAddStorageObligation(so storageObligation) error {
var soid types.FileContractID
err := func() error {
h.mu.Lock()
defer h.mu.Unlock()
// Sanity check - obligation should be under lock while being added.
soid = so.id()
_, exists := h.lockedStorageObligations[soid]
if !exists {
h.log.Critical("addStorageObligation called with an obligation that is not locked")
}
// Sanity check - There needs to be enough time left on the file contract
// for the host to safely submit the file contract revision.
if h.blockHeight+revisionSubmissionBuffer >= so.expiration() {
h.log.Critical("submission window was not verified before trying to submit a storage obligation")
return errNoBuffer
}
// Sanity check - the resubmission timeout needs to be smaller than storage
// proof window.
if so.expiration()+resubmissionTimeout >= so.proofDeadline() {
h.log.Critical("host is misconfigured - the storage proof window needs to be long enough to resubmit if needed")
return errors.New("fill me in")
}
// Add the storage obligation information to the database.
err := h.db.Update(func(tx *bolt.Tx) error {
// Sanity check - a storage obligation using the same file contract id
// should not already exist. This situation can happen if the
// transaction pool ejects a file contract and then a new one is
// created. Though the file contract will have the same terms, some
// other conditions might cause problems. The check for duplicate file
// contract ids should happen during the negotiation phase, and not
// during the 'addStorageObligation' phase.
bso := tx.Bucket(bucketStorageObligations)
// If the storage obligation already has sectors, it means that the
// file contract is being renewed, and that the sector should be
// re-added with a new expriation height. If there is an error at any
// point, all of the sectors should be removed.
if len(so.SectorRoots) != 0 {
err := h.AddSectorBatch(so.SectorRoots, so.expiration())
if err != nil {
return err
}
}
// Add the storage obligation to the database.
soBytes, err := json.Marshal(so)
if err != nil {
return err
}
}
// Add the storage obligation to the database.
soBytes, err := json.Marshal(so)
return bso.Put(soid[:], soBytes)
})
if err != nil {
return err
}
return bso.Put(soid[:], soBytes)
})
// Update the host financial metrics with regards to this storage
// obligation.
h.financialMetrics.ContractCount++
h.financialMetrics.PotentialContractCompensation = h.financialMetrics.PotentialContractCompensation.Add(so.ContractCost)
h.financialMetrics.LockedStorageCollateral = h.financialMetrics.LockedStorageCollateral.Add(so.LockedCollateral)
h.financialMetrics.PotentialStorageRevenue = h.financialMetrics.PotentialStorageRevenue.Add(so.PotentialStorageRevenue)
h.financialMetrics.PotentialDownloadBandwidthRevenue = h.financialMetrics.PotentialDownloadBandwidthRevenue.Add(so.PotentialDownloadRevenue)
h.financialMetrics.PotentialUploadBandwidthRevenue = h.financialMetrics.PotentialUploadBandwidthRevenue.Add(so.PotentialUploadRevenue)
h.financialMetrics.RiskedStorageCollateral = h.financialMetrics.RiskedStorageCollateral.Add(so.RiskedCollateral)
h.financialMetrics.TransactionFeeExpenses = h.financialMetrics.TransactionFeeExpenses.Add(so.TransactionFeesAdded)
return nil
}()
if err != nil {
return err
}
// Update the host financial metrics with regards to this storage
// obligation.
h.financialMetrics.ContractCount++
h.financialMetrics.PotentialContractCompensation = h.financialMetrics.PotentialContractCompensation.Add(so.ContractCost)
h.financialMetrics.LockedStorageCollateral = h.financialMetrics.LockedStorageCollateral.Add(so.LockedCollateral)
h.financialMetrics.PotentialStorageRevenue = h.financialMetrics.PotentialStorageRevenue.Add(so.PotentialStorageRevenue)
h.financialMetrics.PotentialDownloadBandwidthRevenue = h.financialMetrics.PotentialDownloadBandwidthRevenue.Add(so.PotentialDownloadRevenue)
h.financialMetrics.PotentialUploadBandwidthRevenue = h.financialMetrics.PotentialUploadBandwidthRevenue.Add(so.PotentialUploadRevenue)
h.financialMetrics.RiskedStorageCollateral = h.financialMetrics.RiskedStorageCollateral.Add(so.RiskedCollateral)
h.financialMetrics.TransactionFeeExpenses = h.financialMetrics.TransactionFeeExpenses.Add(so.TransactionFeesAdded)
// Set an action item that will have the host verify that the file contract
// has been submitted to the blockchain, then another to submit the file
// contract revision to the blockchain, and another to submit the storage
// proof.
err0 := h.tpool.AcceptTransactionSet(so.OriginTransactionSet)
if err0 != nil {
h.log.Println("Failed to add storage obligation, transaction set was not accepted:", err0)
// Check that the transaction is fully valid and submit it to the
// transaction pool.
err = h.tpool.AcceptTransactionSet(so.OriginTransactionSet)
if err != nil {
h.log.Println("Failed to add storage obligation, transaction set was not accepted:", err)
return err
}
// Queue the action items.
h.mu.Lock()
defer h.mu.Unlock()
// The file contract was already submitted to the blockchain, need to check
// after the resubmission timeout that it was submitted successfully.
err1 := h.queueActionItem(h.blockHeight+resubmissionTimeout, soid)
......@@ -384,7 +398,7 @@ func (h *Host) addStorageObligation(so storageObligation) error {
err2 := h.queueActionItem(so.expiration()-revisionSubmissionBuffer, soid)
// The storage proof should be submitted
err3 := h.queueActionItem(so.expiration()+resubmissionTimeout, soid)
err = composeErrors(err0, err1, err2, err3)
err = composeErrors(err1, err2, err3)
if err != nil {
h.log.Println("Error with transaction set, redacting obligation, id", so.id())
return composeErrors(err, h.removeStorageObligation(so, obligationRejected))
......
......@@ -109,7 +109,7 @@ func TestBlankStorageObligation(t *testing.T) {
t.Fatal(err)
}
ht.host.managedLockStorageObligation(so.id())
err = ht.host.addStorageObligation(so)
err = ht.host.managedAddStorageObligation(so)
if err != nil {
t.Fatal(err)
}
......@@ -202,7 +202,7 @@ func TestSingleSectorStorageObligationStack(t *testing.T) {
t.Fatal(err)
}
ht.host.managedLockStorageObligation(so.id())
err = ht.host.addStorageObligation(so)
err = ht.host.managedAddStorageObligation(so)
if err != nil {
t.Fatal(err)
}
......@@ -375,7 +375,7 @@ func TestMultiSectorStorageObligationStack(t *testing.T) {
t.Fatal(err)
}
ht.host.managedLockStorageObligation(so.id())
err = ht.host.addStorageObligation(so)
err = ht.host.managedAddStorageObligation(so)
if err != nil {
t.Fatal(err)
}
......@@ -613,7 +613,7 @@ func TestAutoRevisionSubmission(t *testing.T) {
t.Fatal(err)
}
ht.host.managedLockStorageObligation(so.id())
err = ht.host.addStorageObligation(so)
err = ht.host.managedAddStorageObligation(so)
if err != nil {
t.Fatal(err)
}
......
......@@ -81,6 +81,8 @@ func (h *Host) initRescan() error {
h.log.Println("dropping storage obligation during rescan, id", so.id())
}
// AcceptTransactionSet needs to be called in a goroutine to avoid a
// deadlock.
go func(i int) {
err := h.tpool.AcceptTransactionSet(allObligations[i].OriginTransactionSet)
if err != nil {
......
......@@ -18,16 +18,21 @@ const (
)
// dustValue is the quantity below which a Currency is considered to be Dust.
//
// TODO: These need to be functions of the wallet that interact with the
// transaction pool.
func dustValue() types.Currency {
return types.SiacoinPrecision
return types.SiacoinPrecision.Mul64(3)
}
// defragFee is the miner fee paid to miners when performing a defrag
// transaction.
//
// TODO: These need to be functions of the wallet that interact with the
// transaction pool.
func defragFee() types.Currency {
fee := types.SiacoinPrecision.Mul64(5)
if dustValue().Mul64(defragBatchSize).Cmp(fee) <= 0 {
return dustValue().Mul64(defragBatchSize)
}
return fee
// 35 outputs at an estimated 250 bytes needed per output means about a 10kb
// total transaction, much larger than your average transaction. So you need
// a lot of fees.
return types.SiacoinPrecision.Mul64(20)
}
......@@ -18,9 +18,6 @@ var (
// throughout scanning the outputs to determine if defragmentation is necessary
// and then proceeding to actually defrag.
func (tb *transactionBuilder) fundDefragger(fee types.Currency) (types.Currency, error) {
tb.wallet.mu.Lock()
defer tb.wallet.mu.Unlock()
// Collect a set of outputs for defragging.
var so sortedOutputs
for scoid, sco := range tb.wallet.siacoinOutputs {
......@@ -150,17 +147,20 @@ func (w *Wallet) threadedDefragWallet() {
return
}
// Create a transaction builder.
// Create a transaction builder and fund it with the outputs to be
// defragged.
fee := defragFee()
w.mu.Lock()
tbuilder := w.registerTransaction(types.Transaction{}, nil)
// Fund it using a defragging specific method.
amount, err := tbuilder.fundDefragger(fee)
w.mu.Unlock()
if err != nil {
if err != errDefragNotNeeded {
w.log.Println("Error while trying to fund the defragging transaction", err)
}
return
}
// Add the miner fee.
tbuilder.AddMinerFee(fee)
// Add the refund.
......
......@@ -35,7 +35,7 @@ func TestDefragWallet(t *testing.T) {
}
// allow some time for the defrag transaction to occur, then mine another block
time.Sleep(time.Second)
time.Sleep(time.Second * 5)
_, err = wt.miner.AddBlock()
if err != nil {
......
......@@ -609,6 +609,8 @@ func (w *Wallet) registerTransaction(t types.Transaction, parents []types.Transa
// most typical call is 'RegisterTransaction(types.Transaction{}, nil)', which
// registers a new transaction without parents.
func (w *Wallet) RegisterTransaction(t types.Transaction, parents []types.Transaction) modules.TransactionBuilder {
w.mu.Lock()
defer w.mu.Unlock()
return w.registerTransaction(t, parents)
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment