• Specify a SecureRandom implementation instead of using the JRE's default
• Use an implementation without insecure fallbacks

• changeOutput.getMinNonDustValue() returns 540, shouldn't it be 543/546?

• SendRequest.tx is modified even when Wallet.completeTx() throws ExceededMaxTransactionSize exception
  bitcoinj/bitcoinj#1359
  This is more likely to be reproduced if we implement the add/remove federators feature and have to send all the federation money to a new federation. Or when we group release txs

• Wallet.completeTx() with P2SH multisig may not detect too big txs
  bitcoinj/bitcoinj#1358
  This is more likely to be reproduced if we implement the add/remove federators feature and have to send all the federation money to a new federation. Or when we group release txs

• Fee refactor: Sync fee calculation with Core 0.14
  bitcoinj/bitcoinj#1356

• Understand why changeOutput.getValue() is added to fee? I can't see any effect of this

• if (changeOutput.getMinNonDustValue().isGreaterThan(changeOutput.getValue())) {
  // Never create dust outputs; if we would, just
  // add the dust to the fee.
  fee = fee.add(changeOutput.getValue());

• When selecting coins to spend, don't select coins that are ""dust to me"" given the feePerKb we use: If to spend an UTXO we have to add an input whose size makes the fee increase bigger than the UTXO value, don't pick that UTXO.

• A change output may not be dust for the network, but may be ""dust to me"". If adding a change output (not dust) makes the tx to pay an extra fee bigger than the change output, throw the change to fees (or make recipients pay the difference to take it out of dust if SendRequest.recipientsPayFees is true). Also, a P2SH output may require a big 10 of 15 multisig script sig, so the size of the script sig required to spend it may be taken into account. Create a boolean SendRequest.avoidDustToMe to implement this feature

• If change output is dust, split the cost of getting out of dust between all the recipients' outputs, not just the first output.

See bitcoinj/bitcoinj#1352

SendRequest.tx is modified even when Wallet.completeTx() throws ExceededMaxTransactionSize exception
bitcoinj/bitcoinj#1359

Wallet.completeTx() with P2SH multisig may not detect too big txs
bitcoinj/bitcoinj#1358
This is more likely to be reproduced if we implement the add/remove federators feature and have to send all the federation money to a new federation. Or when we group release txs
Fee refactor: Sync fee calculation with Core 0.14
bitcoinj/bitcoinj#1356
Understand why changeOutput.getValue() is added to fee? I can't see any effect of this
if (changeOutput.getMinNonDustValue().isGreaterThan(changeOutput.getValue())) {
// Never create dust outputs; if we would, just
// add the dust to the fee.
fee = fee.add(changeOutput.getValue());
When selecting coins to spend, don't select coins that are "dust to me" given the feePerKb we use: If to spend an UTXO we have to add an input whose size makes the fee increase bigger than the UTXO value, don't pick that UTXO.

A change output may not be dust for the network, but may be "dust to me". If adding a change output (not dust) makes the tx to pay an extra fee bigger than the change output, throw the change to fees (or make recipients pay the difference to take it out of dust if SendRequest.recipientsPayFees is true). Also, a P2SH output may require a big 10 of 15 multisig script sig, so the size of the script sig required to spend it may be taken into account. Create a boolean SendRequest.avoidDustToMe to implement this feature
If change output is dust, split the cost of getting out of dust between all the recipients' outputs, not just the first output.

See bitcoinj/bitcoinj#1352

• When selecting coins to spend, don't select coins that are "dust to me" given the feePerKb we use: If to spend an UTXO we have to add an input whose size makes the fee increase bigger than the UTXO value, don't pick that UTXO. 

• A change output may not be dust for the network, but may be "dust to me". If adding a change output (not dust) makes the tx to pay an extra fee bigger than the change output, throw the change to fees (or make recipients pay the difference to take it out of dust if SendRequest.recipientsPayFees is true). Also, a P2SH output may require a big 10 of 15 multisig script sig, so the size of the script sig required to spend it may be taken into account. Create a boolean SendRequest.avoidDustToMe to implement this feature 

• If change output is dust, split the cost of getting out of dust between all the recipients' outputs, not just the first output. 

See 
https://github.com/bitcoinj/bitcoinj/pull/1352 

We removed bitcoin block time validation because it used the computer time in the context of a smart contract and that breaks consensus rules.
See oscarguindzberg/bitcoinj@aec5ed9

We still need some defense from rogue miners putting fake timestamps on btc blocks.

To do: Check the timestamp of the btc block is not more than 2 hours ahead of the timestamp of the rsk block that includes the ReceiveHeaders tx."
See oscarguindzberg/bitcoinj@aec5ed9

We still need some defense from rogue miners putting fake timestamps on btc blocks.

To do: Check the timestamp of the btc block is not more than 2 hours ahead of the timestamp of the rsk block that includes the ReceiveHeaders tx.

Currently is set to several minutes in the future. Ethereum sets it to a value close to 20 seconds (check). We should discuss to set it between 20 seconds and 1 minute.

When a block is executed (no matter it's origin) the flow starts somewhere and arrives to the BlockExecutor, then TransactionExecutor and for every transaction it runs code on the VM. 
Check Exception handling on each of the previous classes. Make special focus on go method from TransactionExecutor and play method from VM.

Current implementation is a bit convoluted (see updateBatch implementation in RepositoryImpl and RepositoryTrack). The cache of ContractDetails kept by DetailsDataStore is SHARED by all Repository objects. The manage of originalContractDetails in updateBatch repository methods is not clear and direct. DetailsDataStore arbitrary close all the contract stores in its cache, when flush 

nitially, BlockExecutor was a simple object that runs the transactions in a block, and in other method, apply the results (root store, etc...) to fill a block. Now, the execution of the transactions is polluted by the use of flags. And it is used with different flags from different core code (miner server vs blockchain). And one flag is used to skip some "not well formed" transactions in old tests. 

We should refactor this class to have a clearer use, and improve to have test with "real and accepted and well-formed" transactions 

Review in relation to storage rent and bitcoin price

Currently, in many places, the global configuration objects SystemProperties.CONFIG and RskSystemProperties.RSKCONFIG (two instances, there is another issue to unify them). These values are used directly in logic: we cannot inject the needed values for testing purposes. These values are used directly in logic: we cannot inject the needed values for testing purposes. And we cannot create new empty config objects: the empty constructor has logic to retrieve their values from some other place. These two objects are global singletons, and their use should be refactored

Instead of relaying on usage counter (not all the use cases of a LevelDB are closed, ie some ContractDetailsImpl uses), we should keep a pool by usage (time of last activity)

The idea is to automate the 2WP process explained in the Wiki

The description of the API method is at https://github.com/ethereum/go-ethereum/wiki/Management-APIs#personal_importrawkey 

Apparently, the geth node allows the immediate use of this account. In our implementation, the account is saved, but should be explicitly unlocked using unlockAccount and passphrase

See https://github.com/bitcoin/bips/blob/master/bip-0173.mediawiki

When a transaction specifies a destination address, it can be serialized with less than 20 bytes. If so, when processing the tx, expand the address to 20 bytes BEFORE checking anything in the trie. Also when a transaction is checked for its correct nonce during relay, the address must be expanded.

These classes could be replaced by a normal blockchain. There are two SimpleBlockchain classes.

Many unit tests fail in Windows environment due to a bug in the version we use of MapDB

There are currently many classes triggering threads to execute different tasks (e.g. the new PendingState implementation triggers a task to remove obsolete transactions). This make it very difficult to manage all threads opened by the application. There should only be one class working as a scheduler.

Currently the extradata field can have any size. Check this. If so, the extradata size must be restricted. 

Currently it does nothing (ie keeps the sBTC and does not release the BTC). See how we handled this same use case in Remasc.

After updating StateTestRunner to use NewBlockChain, there is a test in TransactionTest that fails 

It is qualified with an @ignore. The test is constantCallConflictTest 

• Implement stronger authentication mechanisms while keeping compatibility with web3js clients

The old BlockchainBuilder returned an old BlockchainImpl. Now, it returns a NewBlockChain. But its purpose is like the new BlockChainBuilder. Replace by this class.

We now do org.bitcoinj.core.NetworkParameters params = org.bitcoinj.params.RegTestParams.get(); 

Use value taken from BridgeConstants 

There are two global configuration objects SystemProperties.CONFIG and RskSystemProperties.RSKCONFIG Both are instantiated at the beginning andonly one is updated by CLIInterface (the command line interface). We should use only one. 

The Ethereum consensus rule (parent.getTimestamp() > block.getTimestamp()) is not enforced

When mining, the block timestamp must be set to the maximum between the current time and the previous block time plus one. 

Some tests belong there

Web3Impl.eth_newFilter() allows you to specify a block range ro receive notifications of past events. 
The code iterates all the blocks in the range and all the transactions looking for those matching the filter. 
This will take forever in production. 

Geth node uses a system inspired by Mipmap (https://en.wikipedia.org/wiki/Mipmap) where there is a bloom filter stored every 1 million blocks, another every 1 thouthands blocks, etc. So, you can quickly discard groups of blocks where there is no relevant information. 
See 
https://github.com/ethereum/go-ethereum/blob/c72f5459ac110de47fa068bfa560164078508b1e/core/database_util.go#L488-L522 
https://github.com/ethereum/go-ethereum/blob/c72f5459ac110de47fa068bfa560164078508b1e/eth/filters/filter.go#L98-L126 

• Implement strong key derivation function.
• Implement authenticated encryption.
• Enforce passphrase strength
• Enforce secure file permissions
• Do not use Java serialization

The second createBlock method creates a block, but overriding some methods using an anonymous class, using a "hardcoded header". It should be removed.

This is currently a constant value. It could be adjusted according to certain parameters (tx amount, etc.)

We use RskAllowUnconfirmedCoinSelector which sort utxos pseudorandomly (tx hash + utxo index).

We were using AllowUnconfirmedCoinSelector which selects bigger UTXOs first, but there are some patterns of UTXO sets and spendings where we might end up with a lot of tiny utxos.
DefaultCoinSelector prioritizes coins days destroyed (coin vale * utxo age) but currently we don't store utxo age information (we could).

utxo selection is a research field. See this talk at scaling bitcoin 2016
https://scalingbitcoin.org/transcript/milan2016/coin-selection
https://www.youtube.com/watch?v=_Z0ID-0DOnc"

We were using AllowUnconfirmedCoinSelector which selects bigger UTXOs first, but there are some patterns of UTXO sets and spendings where we might end up with a lot of tiny utxos. 
DefaultCoinSelector prioritizes coins days destroyed (coin vale * utxo age) but currently we don't store utxo age information (we could). 

utxo selection is a research field. See this talk at scaling bitcoin 2016 
https://scalingbitcoin.org/transcript/milan2016/coin-selection 
https://www.youtube.com/watch?v=_Z0ID-0DOnc 

Separate the implementation of modules exposed via web3 in different Java classes. Currently, all methods are exposed from one class (with inheritance from the original web3impl)