Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Installation scripts for daemon mode on Ubuntu using Upstart #129

Conversation

msgilligan
Copy link

This work is based upon upstream work in bitcoin core on the master branch and in some PRs.
The "Cherry Pick" commit is here: msgilligan@55a97b9

All files (both the "cherry pick" files and the ones that I've added are in the contrib subdirectory.

@m21 m21 closed this Sep 16, 2014
# Pay an optional transaction fee every time you send bitcoins. Transactions with fees
# are more likely than free transactions to be included in generated blocks, so may
# be validated sooner.
paytxfee=0.00
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems risky. I already raised the hardcoded default, but overwriting this here annihilates the effect.

Fees are chosen this way: max(paytxfee, mintxfee)

I suggest to either #comment this or use a sane minimum such as 0.0001 or 0.00001 -- just for the case the mintxfee is for some reason overwritten somehow.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @dexX7 I will fix this in my branch.

I created this pull request to get exactly this kind of feedback, but it has been closed without comment.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not in contact with @m21, but I think this was no manual close, but related to the 0.0.7 release and the branch restructure - mastercoin-MSC:michael-0921 was deleted and I assume this disconnects associated PRs?

The new dev branch is https://github.com/mastercoin-MSC/mastercore/tree/0.0.8 and you might keep an eye on #130 where I suggested to use a more telling branch/release name - just for the case 0.0.8 will be deleted as well.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hope that is the case. I can resubmit on a new branch.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @dexX7, why did you raise nMinRelayTxFee from 1000 to 1001? (I'm working on submitting a new PR and want to incorporate your suggestions and understand the issues better.) It seems to me that the best solution is to comment this line out, though.

This bitcoin.conf file should be the default for Master Core and should be secure and have the best general purpose settings for bitcoind running in background mode. I'm happy to take any suggestions -- or complete configuration files -- for what its contents should be.

(Note: I created this bitcoin.conf file by copying contrib/debian/examples/bitcoin.conf and adding txindex=1 as required by Master Core.)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this is already clear, but just to be sure some background information which might be handy to know: there is a difference between min-X-fee and X-fee. The min-X-fee defines the absolute minimum whereby the X-fee is never less than the min-X-fee. The hardcoded default is overwritten by a config value. To be more specific: there is the mintxfee which is either defined by the client's hardcoded default or config/startup arguments and there is the paytxfee which can be set via config/startup as well, but also on the fly via RPC command settxfee. The actual fee is max(mintxfee, paytxfee). The relayTxFee has a hardcoded default and can be set via config/startup arguments, too. - Unfortunally there is no RPC call to change this value on the fly.

Related to nMinRelayTxFee: at the beginning a magic number was used for all output amounts and this number was changed from time to time to identify and differentiate between clients and versions. This is currently done with Omni as well and you can identify transactions that are created by Omni on it's output amounts of 0.00005757. Switching to an approach which yields output amounts derived from script length and based on minRelayTxFee sort of killed the ability to "tag" clients, but I later realized it's still possible, even without static magic number. I used 1001 basically to make my point and suggested to change this number, if a new "tag" is required.

Given how the nMinRelayTxFee is [ab]used for client identification purposes, I think commenting the line in bitcoin.conf is the way to go at the moment.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the explanation @dexX7! The comment to this line has been added to my branch and is in the updated and re-submitted PR.

@msgilligan
Copy link
Author

@m21 Please re-open this pull request. The code may not be ready to be merged, but I would like to get feedback on it and continue to work on it until it is ready for inclusion.

cc: @CraigSellars @zathras-crypto @faizkhan00 @achamely @marv-engine

@msgilligan
Copy link
Author

@dexX7 says this might have been an automatic close related to some restructuring. If so, I can resubmit the PR to the new dev branch.

@m21
Copy link

m21 commented Sep 16, 2014

I hope this is good for everyone as the new dev branch , targeting the next release: https://github.com/mastercoin-MSC/mastercore/tree/mscore-0.0.8

@msgilligan
Copy link
Author

One of the upstream PRs was just merged:
bitcoin#4611

I also opened an upstream issue for an updated Debian package:
bitcoin#4935

@msgilligan
Copy link
Author

Will resubmit an updated version of this patch to mscore-0.0.8

@msgilligan
Copy link
Author

This PR has been resubmitted as #132 (it is up-to-date with score-0.0.8 and has @dexX7 's suggested comment out of the paytxfee setting.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants