factom-asset-tokens / fatd Goto Github PK

The RPC endpoint path is currently /v1, however get-daemon-properties returns { fatdversion: 'r162.3d7f272', apiversion: '0' }

Change hard-coded string "0" to "1": https://github.com/Factom-Asset-Tokens/fatd/blob/develop/srv/methods.go#L508

To reproduce from a clean database directory, develop commit 273adaa88e9e2d2e62822241c71e14c29ebd84ff:

1. ./fatd -ecadr <ec> -debug -networkid=0x883e093b -dbpath <path> -whitelist 0cccd100a1801c0cf4aa2104b15dec94fe6f45d0f3347b016ed20d81059494df 962a18328c83f370113ff212bae21aaf34e5252bc33d59c9db3df2a6bfda966f
2. Wait until sync completed
3. Send a normal FAT-0 transaction on chain 0cccd100a1801c0cf4aa2104b15dec94fe6f45d0f3347b016ed20d81059494df, for example entryhash 7524c9ec2b7ceeb7494d581a16af2ff72c47842ec1df02313ae3d5b1f8450e46, without pending transactions
4. Hit control+c to interrupt the fatd process
5. Get the panic debug from #31
6. Restart fatd with the same command as above
7. Get this panic: https://gist.github.com/drkatz/51358eed19fa6c65fe72d3bcb3cb5bb4

Every build should have Version, Commit, and build date/time added to the build.

We can use the following build flags for this:

go build -ldflags "-X flag.Commit=$COMMIT flag.Version=$VERSION flag.BuildDate=$(date)"

The COMMIT and VERSION variables can be populated by parsing git calls. The commit string should indicate whether there were uncommitted changes that were part of the build. A "+" after the commit can be used to indicate that.

These variables then need to be made available to the user using the fatd RPC method "get-daemon-properties" and be available using a command like fat-cli version.

To reproduce from a fresh database directory, develop commit 273adaa88e9e2d2e62822241c71e14c29ebd84ff:

1. ./fatd -ecadr <ec> -debug -networkid=0x883e093b -dbpath <path> -whitelist 0cccd100a1801c0cf4aa2104b15dec94fe6f45d0f3347b016ed20d81059494df 962a18328c83f370113ff212bae21aaf34e5252bc33d59c9db3df2a6bfda966f
2. Wait until sync completed
3. Hit control+c to interrupt the fatd process
4. Get this: https://gist.github.com/drkatz/3087dc7be81d29abb452614391a5ce76

Implement a consistent retry mechanism on all factomd API calls in engine.
Exponential backoff with some sane limits should be used. This package is recommended: https://github.com/cenkalti/backoff

Retries should only occur if we get back a 500 server error. At least one successful attempt to hit the factomd API should be required. So don't retry during initial setup in the engine.Start function.

Errors should be logged and the next retry time should be printed.

The -factomscanretries flag should be renamed but we should still have a similar flag.

A context should be used so that stop signals are responded to.

$ ./fat-cli --version
fat-cli:  v0.4.2.r38.ge5c9222
fatd:     v0.4.2.r38.ge5c9222
fatd API: 1

Adding -m '"coinbase TX"' to a transact command doesn't result in the transaction having this meta data attached. I can tell the value of the metadata flag is parsed because I can get an "invalid JSON error" if the value is wrong. But if correct, it seems to simply never make it to the actual transaction.

Walletd supports full wallet.db encryption. Fatd should support walletd encryption in order to fully cater to users that require heightened wallet security, such as exchanges.

Along the lines of #8 (but less critical), having a method returning pending transactions would allow to enhance the user experience in the wallet. It would allow the user to immediately see that his transaction was submitted and is in pending (new block) state.

Currently the api package has all of the Params types for querying the fatd API methods, but not all of the results are put into dedicated types.

Ideally all Params and Results types will be defined, and well documented.

Higher level functions on the provided Client that use the proper Params and Results types are also desired. These functions may then be used by fat-cli.

If get-nf-balance is called for an address that doesn't own any NF token the method return ALL the NF tokens in circulation. If the address own some NF tokens then it returns the correct result.

Have an API similar to factomd multiple-fct-balances
that would return the temporary balance of an address (acknowledged balance vs saved).

Motivation

This is very useful for the end user to have the temporary balance of an address before the block is saved. That allows all UIs to be "reactive" to transactions, otherwise having to wait up to 10 minutes to see what has happened is not a good user experience.

How it would work I think would be for fatd to poll the pending entries, filter any FAT transactions and apply those on top of the saved state.

Is your feature request related to a problem? Please describe.
Currently fat-cli uses the cobra cli framework and uses unix style flags, but fatd uses just the plain "flags" package and some custom parsing. This means that there is some code duplication for common flags (which may be unavoidable in some places anyway) but mainly this creates an unfortunate user experience when switching between the two.

Describe the solution you'd like
Re-implement the fatd cli parsing to use the cobra cli framework similar to fat-cli.

Steps to reproduce on commit 273adaa88e9e2d2e62822241c71e14c29ebd84ff:
1.Sync fatd from scratch: ./fatd -ecadr <ec> -debug -networkid=0x883e093b -dbpath <path> -whitelist 0cccd100a1801c0cf4aa2104b15dec94fe6f45d0f3347b016ed20d81059494df 962a18328c83f370113ff212bae21aaf34e5252bc33d59c9db3df2a6bfda966f
2.Call get-issuance with "includepending":true
3. Get this panic: https://gist.github.com/drkatz/31b8e74f993818aca2281a58c6219283

Add ability to query transaction history of multiple addresses at once and have the pagination working for that set of addresses.

From FAT Discord:

luap

@AdamSLevy I'd like to discuss the possibility to have an API to query the past transactions of multiple addresses at once with pagination (for use in the wallet). Currently I can query for the past transactions of a single address and paginate for that address. But in the wallet I have multiple addresses of the user, and paginating properly the aggregation of the result of individual addresses is a nightmare. I believe it would be straight forward and much more efficient to implement in fatd (as it would simply be a small modification of your SQL query?)

AdamSLevy

good idea.
I can add that.
Easy to add the sql
Can we change the params to accept an array of addresses even if it's a single one?
So I remove the "address" field and replace it with "addresses" which is an array
I could support both actually if you think that's valuable.
But I'd make them mutually exclusive.

luap

always array is fine to me

Maybe an API method that returns the last block processed and the latest Factom block. That way the end user can compute the % of synchronization of the fatd instance he's talking to (or even just know if fatd is fully synced or not).

Add support for new Factom digital identity keys. Those are also on-chain identities with identity keys which follow a slightly different format idpub.../idsec.... Also we should think of which key of an identity to use (currently we arbitrarily chose sk1 for instance).

The FAT Wallet supports only those new identities so until fatd support them too it's impossible to use the wallet to issue a new token or emit coinbase transactions.

Describe the bug
On a fat daemon with one fat-0 and one fat-1 whitelisted chain, sending a pending transaction on the FAT-0 chain results in the FAT-1 chain ID being returned in the result JSON

Describe the environment (please complete the following)

• OS: Ubuntu 18.08
• Factom Network: testnet
• fatd State: synced
• fatd Version: v0.6.0.r71.ge269f60

To Reproduce
Steps to reproduce the behavior:

1. start fatd
2. Wait for sync
3. Send FAT-0 transaction wiht pending true
4. Observe incorrect chain ID in response JSON

Expected behavior
FAT-0 chain ID is returned in request

Is your feature request related to a problem? Please describe.
There are parts of the daemon that do not shut down immediately when a SIGSTOP is received. For example, the done channel is only checked once each DBlock loop, which may be blocked on network or database calls. If context is used throughout, many of these long running function calls can be canceled early to allow for quick exit.

Describe the solution you'd like
Use context.WithCancel at the main of the program and pass this through to any goroutines like the engine and srv. Use this in place of the currently used done channel. Pass this context through any database queries and network calls.

Describe alternatives you've considered
This is the idiomatic way to achieve cancellation.

Additional context
This will require modifying some client libraries like factom and jsonrpc2 to be able to accept a context.Context.

Discussed on Discord:

• Rename parameter start to page. Reduce possible confusion as entryhash is the real starting point.
• Add a parameter for ability to get newest transactions first (i.e choosing ordering of the results).
• Keep default result ordering as "oldest first".

It would be helpful to have an API call that lists all tokens associated with an address. I am thinking about this when considering FAT integration into the TFA explorer. I want to be able, given an FCT address, see all the tokens that it holds. It would also be beneficial to include 0 balances that were once not 0.

This is a quicker process than querying a balance for each possible token on an address.

The API params should just be an FCT address
The result should include the balance, the token name, and chainid of each token the owner holds. In the case of an nf token, the balance field might not be appropriate?

Is your feature request related to a problem? Please describe.
No, this issue requests adding new functionality not based on an existing issue or bug.

Describe the solution you'd like
Add method(s) to the existing host interface supplied to FAT-0 WASM contracts to allow them to call other contracts established on other Factoid Addresses on the same FAT-0 chain.

Currently contracts are isolated, and can only access functions that are internally available in their own WASM module code. This change will allow a contract's WASM code to call externally available ABI exported functions in other contracts, and by extension, access their state. This will enable smart contracts to be built upon each other in a composable way and allow them to interact in more complex ways than is currently allowed.

Examples where this functionality is required:

• Check a condition is satisfied in an existing contract
• Check the balance of a "virtual" FAT token (A token implemented in WASM code on top of an existing FAT token)
• Interact with or get data from a bridge contract that maintains and validates the state of another blockchain (Ethereum bridge for example)

The following will be required:

• Create a host interface function to execute a cross contract call, for example consider the function signature call(<Contract Factoid Address>, <Function Name>, <Function Arguments[]>) executable by a WASM contract
• Create a host interface function to allow a contract to check whether a Factoid address is an established contract. This allows contracts to control and regulate behavior when called by another contract

Standards Affected:

• FAT-105 Host Interface Standard: Specifies what host methods are available to a contract. Part of larger smart contract pull request as part of Factom Protocol grant
• FAT-0 Will also need to be modified to be explicit about how cross contract calls are handled and validated when it comes to maintaining token state.

Affected files within FAT Daemon (Not exhaustive):
internal/runtime:

• context.go - Manages the execution context for calling contracts, used by imports.go to return the proper values and when calling host methods
• imports.go - Lays out internal workings of host functions, function to call function in another contract and function to check if the current call is originated from a contract will need to be added. Call to execute function in another contract will need to reference runtime.go and create new instances to execute chained calls
• runtime.syms - Add function signatures added to imports.go

Describe alternatives you've considered
Use Ethereum 😢

Additional context
The addition of this feature will be carefully considered and spec'd out before implementation, as it represents a myriad of potential vulnerabilities that we've seen in systems such as Ethereum that enable similar functionality. This issue description is only meant as a rough outline and will be amended over time.

Before this functionality is implemented a Gas system will need to be devised to regulate the expense of executing:

• Intentionally or unintentionally malicious expensive contract calls
• Naturally expensive legitimate contract calls
• Circular or recursive contract calls

FAT is intended to allow segregation of computation via Factom Chains. This addition adheres to this philosophy as the token state remains constrained to a single token chain. This allows the --whitelist and --blacklist command line flags to remain part of FAT daemon functionality and allows users to only need to download, validate, and calculate the state for contracts that they are interested in.

Describe the bug
Some time after starts up the daemon crashes with a sqlite related error

Describe the environment (please complete the following)
OS: Ubuntu 18.08
Factom Network: testnet
fatd State: synced
fatd Version: v0.6.0.r71.ge269f60

To Reproduce
Steps to reproduce the behavior:

1. Start fatd ./fatd -ecadr <EC> -debug -networkid=test -dbpath /home/devon/git-projects/fatd/fat.db -whitelist 0cccd100a1801c0cf4aa2104b15dec94fe6f45d0f3347b016ed20d81059494df,962a18328c83f370113ff212bae21aaf34e5252bc33d59c9db3df2a6bfda966f
2. Wait for sync
3. Wait some time, usually < 3 mins
4. Panic occurs

Expected behavior
Normal operation

Output
https://gist.github.com/drkatz/7383be65cc37081bcdc05f49538735c6

Additional context
none

curl -X POST --data-binary '{"jsonrpc": "2.0", "id": 0, "method":"get-nf-balance", "params": {"chainid":"2c6bd47e73e2eed18fa55d622c0d12c9e3b2db8ecf6e8ea75fe001350d490678", "address": "FA2tK2UuXXxisZjpKeCaRJkZgg6nbnoe6uqwQdNFrGiewtXAWbV4", "limit": 5000, "page": 0, "order": "asc"}}' -H 'content-type:text/plain;' http://0.testnet.fat.dbgrow.com:8078

{"jsonrpc":"2.0","error":{"code":-32602,"message":"Invalid params","data":"\"order\" value must be either \"asc\" or \"desc\""},"id":0}

This queries testnet fatd node that is on vesion v1.0.0.r0.g06f44ec
The issue is that I am querying page 0 (instead of page 1) but the error is misleading so it's hard to figure out what is the problem.

get-nf-balance currently always returns the expanded version of token ranges.
User of this API would benefit to get back the short version of a range (i.e {"min": 29, "max": 299}) to not have to go through many pages to retrieve the token list.

@drkatz reported on discord that "During issuance, for the -type argument the docs say the default is "FAT-0" but when omitted it returns an error that says the command is missing the argument"

The expected way to create a coinbase transaction is as follows:

fat-cli -chainid ... transact -coinbase 5 -output FA...:5 -sk1 sk1...

However if a user creates a transaction using the coinbase address as an input, like this:

fat-cli -chainid ... transact -input FA1zT4aFpEvcnPqPCigB3fvGu4Q4mTXY22iiuV69DqE1pNhdF2MC:5 -output FA...:5

then fat-cli will attempt to sign this like a normal transaction. Normally a user would not have the coinbase address in their wallet, as it not a secure address for real use. But the error would be along the lines of "address not found in wallet" which is misleading. Additionally, if the user does have the coinbase address in their wallet then fat-cli will go onto sign the transaction with the coinbase address private key, which produces an invalid coinbase transaction (it must be signed by the issuer's identity key). However since fat-cli treated this as a normal transaction, it did not bother to populate the issuer's public identity key, so this is all zeroes, which is the coinbase private key. This then passes fat-cli's final validation of the transaction and so it is sent to fatd where it will get rejected.

fat-cli should check if the transaction is a coinbase directly, instead of relying on whether or not the -coinbase flag was supplied. Improved error output would also make this more user friendly.

Thank you @drkatz for actually doing the unexpected thing and putting the coinbase address into your wallet, and for reporting this on discord.

Is your feature request related to a problem? Please describe.
Currently fat-cli only submits entries directly to the factom blockchain. It does not allow a user to make use of the "send-transaction" fatd API method.

Describe the solution you'd like
When the user does not supply an --ecadr argument to fat-cli transact ..., attempt to use the "send-transaction" end point. When the user supplies an --ecadr argument, submit the entry directly to factomd, as is currently implemented. Additionally, allow the --curl option to output the proper curl command to submit to the tx to fatd, instead of factomd, if --ecadr is not supplied.

Additional context
The changes should be made to cli/cmd/transact*.go

According to FAT-103, valid entries should have signatures of the following hash:

sha512(<RCD/Sig id salt string>|<time stamp Unix Seconds string>|<Chain ID bytes>|<Tx Content JSON bytes>)

Where <RCD/Sig ID Salt string> is a number encoded as a decimal string indicating the ordering of the RCD/Sig pair in the External IDs of the entry. See FAT-103#Salting.

The current implementation of this signature scheme computes the max length of the data to hash, which depends on the string length of the max RCD/Sig pair ID salt, so it can pre-allocate the data to hash. For example, if there are 11 inputs for a transaction then it will need 11 RCD/Sig pairs and the highest ID salt will be 10 (0-based index), making the string length for that salt 2 bytes. When generating the first 10 signatures, the RCD/Sig pair ID is a single digit, and the first byte in the pre-allocated space should be omitted from the hashed data. The current implementation does not omit that first byte for such signatures, and thus hashes the first byte in the pre-allocated buffer, which may be zero.

For example, if there are 11 inputs, the hashes are currently computed as follows:

sha512(0x00|"0"|<time stamp Unix Seconds string>|<Chain ID bytes>|<Tx Content JSON bytes>)
sha512(0x00|"1"|<time stamp Unix Seconds string>|<Chain ID bytes>|<Tx Content JSON bytes>)
...
sha512(0x00|"9"|<time stamp Unix Seconds string>|<Chain ID bytes>|<Tx Content JSON bytes>)
sha512("10"|<time stamp Unix Seconds string>|<Chain ID bytes>|<Tx Content JSON bytes>)

It should instead be:

sha512("0"|<time stamp Unix Seconds string>|<Chain ID bytes>|<Tx Content JSON bytes>)
sha512("1"|<time stamp Unix Seconds string>|<Chain ID bytes>|<Tx Content JSON bytes>)
...
sha512("9"|<time stamp Unix Seconds string>|<Chain ID bytes>|<Tx Content JSON bytes>)
sha512("10"|<time stamp Unix Seconds string>|<Chain ID bytes>|<Tx Content JSON bytes>)

When the transaction only requires 10 or fewer signatures, then all RCD/Sig pair ID salts are single digits and there is no extra byte allocated.

This means that any signatures generated for transactions with more than 10 inputs should be considered invalid. Unfortunately the implementation that validates the signatures suffers the same issue, so such signatures are viewed as valid by the current fatd implementation.

User Action:

Until a patch is released, refrain from making any transactions with more than 10 inputs.

Any token chain databases that include such transactions are corrupted and should be removed and regenerated with the latest version of fatd once a fix is released. This can be done by simply removing the ./fatd.db directory as a whole.

If you are absolutely confident that certain chain databases that you are tracking have no such transactions, then you can leave those in place and solely remove the chain databases within the ./fatd.db/ directory. Then restart fatd using the -startscanheight flag using the block where the chain in question is first created.

Right now querying page 0 and page 1 is allowed and return the same result. I think this behavior is a risk of confusion and mistake for the user that wouldn't pay attention enough (like myself).

Let's go with a stricter 1based pagination:

• Default page to 1
• Reject a page of 0 (and small of course)

Describe the bug
Calling get-balances causes a panic

Describe the environment (please complete the following)

• OS: Ubuntu 18.01
• Factom Network: testnet
• fatd State: synced
• fatd Version: v0.6.0.r84.g90575c4
• fat-cli Version: v0.6.0.r84.g90575c4

To Reproduce
Steps to reproduce the behavior:

1. ./fatd -ecadr EC1tE4afVGPrBUStDhZPx1aHf4yHqsJuaDpM7WDbXCcYxruUxj2D -debug -networkid=test -dbpath /home/devon/git-projects/fatd/fat.db -whitelist 0cccd100a1801c0cf4aa2104b15dec94fe6f45d0f3347b016ed20d81059494df,962a18328c83f370113ff212bae21aaf34e5252bc33d59c9db3df2a6bfda966f
2. Wait for sync ...
3. Run fat-js test suite
4. Experience non-fatal panic

Expected behavior
Normal operation

Output
https://gist.github.com/drkatz/e49d19778dfa3fa81e26247917e8ab39

Additional context
This happened once and have not had luck reproducing yet. Perhaps the debug can help find the issue

Describe the bug
Random 500 ISE related to checking for pending txs occurs causing fatd to panic and crash

Describe the environment (please complete the following)

OS: Ubuntu 18.01
Factom Network: testnet
fatd State: synced
fatd Version: v0.6.0.r84.g90575c4
fat-cli Version: v0.6.0.r84.g90575c4

To Reproduce

1. ./fatd -ecadr EC1tE4afVGPrBUStDhZPx1aHf4yHqsJuaDpM7WDbXCcYxruUxj2D -debug -networkid=test -dbpath /home/devon/git-projects/fatd/fat.db -whitelist 0cccd100a1801c0cf4aa2104b15dec94fe6f45d0f3347b016ed20d81059494df,962a18328c83f370113ff212bae21aaf34e5252bc33d59c9db3df2a6bfda966f
2. Wait for sync ...
3. Wait for indeterminate panic

Expected behavior
Normal behavior

Output
https://gist.github.com/drkatz/b279c4d855300feb1d0af131ee368bf2

Additional context
Seems to happen randomly?

$ curl -X POST --data-binary '{"jsonrpc": "2.0", "id": 0, "method":"get-issuance", "params":{ "chainid":"70e42beed688c97950777b371b88f94d06db857acc9bb09a010197e541126eef"}}' -H 'content-type:text/plain;' http://0.testnet.fat.dbgrow.com:8078

{"jsonrpc":"2.0","result":{"chainid":"70e42beed688c97950777b371b88f94d06db857acc9bb09a010197e541126eef","tokenid":"Luap Coin","issuerid":"8888889822cf1d5889aa8dc11ad210b67d582812152de568fabc5f8505989c0f","entryhash":"54d0ee5153d6de97022904ed1378dae547f6bbf9af7fe1eb922b537ea2687b54","timestamp":1555462200,"issuance":{"type":"FAT-0","supply":1000000,"symbol":"LUAP"}},"id":0}

54d0ee5153d6de97022904ed1378dae547f6bbf9af7fe1eb922b537ea2687b54 seems to be incorrect as it is an entry from the identity chain. At the time of this issue, the version of the testnet courtesy node is v0.4.2.r4.ge13e2aa

Describe the bug
Sqlite error occurs while running fat-js test suite

Describe the environment (please complete the following)

OS: Ubuntu 18.01
Factom Network: testnet
fatd State: synced
fatd Version: v0.6.0.r93.g183fbee
fat-cli Version: v0.6.0.r93.g183fbee

To Reproduce

1. ./fatd -ecadr -debug -networkid=test -dbpath /home/devon/git-projects/fatd/fat.db -whitelist 0cccd100a1801c0cf4aa2104b15dec94fe6f45d0f3347b016ed20d81059494df,962a18328c83f370113ff212bae21aaf34e5252bc33d59c9db3df2a6bfda966f
2. Wait for sync
3. run fat-js integration tests against fatd
4. experience this error eventually, happens on startup at times

Expected behavior
Normal behavior

Output
https://gist.github.com/drkatz/a59c462733d571acf73b0ea6503fddfd

Additional context
Once this happens the database must be cleared, otherwise this happens on each startup

Make fatd set HTTP headers specifying its commit version & API version in RPC requests. Useful for clients to preemptively detect compatibility issues without needing to parse a potentially incompatible request body.

It would be useful to have the number of on-zero balance addresses for a given token included in the result of the get-stats RPC call. This is typically a sign of health for token economies and is often displayed in explorers.

root@fatd:~# lsb_release -a
Distributor ID: Ubuntu
Description:    Ubuntu 18.04.2 LTS
Release:        18.04
Codename:       bionic

root@fatd:~# ldd --version
ldd (Ubuntu GLIBC 2.27-3ubuntu1) 2.27

root@fatd:~# ./fatd
./fatd: /lib/x86_64-linux-gnu/libc.so.6: version `GLIBC_2.28' not found (required by ./fatd)

Ubuntu Cosmic or Debian Buster(testing) seem to be fine.