Skip to content

Conversation

@weilu
Copy link
Contributor

@weilu weilu commented Jun 17, 2014

This PR 1) fixes the constants used for Bitcoin transaction fees & dust and 2) adds fee estimation for dogecoin and litecoin as an example of how it can be done for alternative networks.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.0%) when pulling 5543f14 on weilu:feendust into e2cf354 on bitcoinjs:master.

test/network.js Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you use the transactions from fixtures/transaction.json instead? I'd prefer to remove mainnet_tx.json if we can.
Or better yet, make test fixtures just for network.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 5dcefc5. I removed its dependency on fixtures entirely and used stubs instead. After all, why should fee estimation care about how transaction is serialized? It assumes tx.toBuffer does its job correctly and produces the correct size.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed. I think the stubs are a solid approach in this case. Might be a good idea to eventually move the test cases themselves to a more data driven approach though, if possible.

src/networks.js Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

See #216. If that gets merged, please change dustsofthreshold to dustSoftThreshold

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done with a rebase, which also cleaned up the commit history

dcousens added a commit that referenced this pull request Jun 17, 2014
@dcousens dcousens merged commit 9e2e0bd into bitcoinjs:master Jun 17, 2014
@weilu weilu deleted the feendust branch June 17, 2014 15:42
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.

3 participants