mysteriumnetwork / payments Goto Github PK
View Code? Open in Web Editor NEWGolang package/SDK for Mysterium Network payments system
License: GNU Lesser General Public License v3.0
Golang package/SDK for Mysterium Network payments system
License: GNU Lesser General Public License v3.0
In order to be able to fix them in the future
At the moment we're seeing quite a few issues with transactions being sent but not reflected in the network.
Since this issue seems to be global, need a global way of tracking transactions for both accountant and transactor.
Requirements:
On sending a transaction, the state of the transaction should be checked. If after a period of time the transaction is still not reflected in the network - retry the transaction with the same nonce.
Missing tests:
sched_offchain_test.go
could be improved after recent offchain changes.transaction
could use a few more tests.gas
could use a few more tests.units
has no tests.registration
has no tests.exchange
is very heavily used in multiple places and has little to no tests. Having places like currency.go
or api.go
tested so that they're most or less set in stone, would be nice.client
is missing some tests, for example for addresses.go
crypto
and check if anything is missing, a lot of is covered but it has also not received any tests for a long time now.gas/etherscan
or gas/matic
Produce a simple service with an attached redis storage which would be able to cache gas prices for some chains and expose them through an API. Reasoning for this:
Services should still be able to send transaction if our gas station is out.
Bonus: The API should have a rate limit if no key (auth) is provided
Add metrics for the fees and transactions delivered. Would be nice to see in a chart how much txs each service is doing and how much it is paying for it
At the moment IsPromiseValid
and RecoverSigner
are using func ReformatSignatureVForRecovery(signature []byte) error
which is mutating promise object signature.
The exchange message has a weird method name, ValidateExchangeMessage. Should be something along the lines of IsMessageValid, IsMessageOK or what not.
After a certain period of time, lets say 30 days, those entries are just simply useless. If they are delivered and are old, they should be removed.
This would decrease risk of getting cheques (promises) which are not covered by any funds because sender did withdrawal faster that receiver did claim his cheque.
It would be nice to have tests for some parts, especially depot and courier at least,
It became clear that our company is going to need some user authentication in order to provide services to users. Namely, it can be useful for following applications:
All it needs to do is to authenticate user using one of configured methods (e.g. plain password) and return JWT token (similar to OAuth 2.0). Services which need to authenticate user will just validate access token and allow access granted for this user.
Note that one user may have multiple confirured authentication methods like password, social account login and so on. For starters lets make only password option, but keep that in mind designing data schema.
Also note that users may belong to different namespaces: i.e. registation database (realm) for webscraping may be different than the one for the VPN service. These accounts are completely different for these services.
Also we will need usual stuff like password resets etc.
It is also possible to consider off-the-shelf options like IdentityServer, but I doubt these are customizable enough, so we'd probably start with own simple thing.
Other services with their own custom observer API implementation should drop it and use the generic observer provided by payments package.
To potentially increase our throughput and make the queue easier to use, we could queue transactions in to separate queues per chain.
At the moment service consumer issued promises are subject to possible replay attacks:
There are places in payments where we use context with no timeout resulting in forever hanging BC calls or client creations.
We should have a single thread which we can re-use in all our code. It should be able to to submit transactions in order and return they data to the caller.
This would eliminate the need for custom nonce trackers and rude nonce issues, which are hard to keep up with sometimes.
Include logging in relevant places to try and pin point the issues
Currently something similar exists in node but it is not even close to good enough. We would like to define a custom type which would unify our representation of an address.
Most services now call strings.ToLower(Address.Hex())
when needing to compare these addreses in DB funcs or return them from public facing APIs. Examples include but are not limited:
What we want:
common
package of eth library we useWe need to change the data types for sequenceID and amount in the promise to uint64
https://github.com/mysteriumnetwork/payments/blob/master/promises/promise.go#L31
Hello,
I am very new to this codebase, so I could be easily mistaken, but I believe that the withdraw(...)
function in IdentityBalances.sol
is vulnerable to a replay attack. And this vulnerability allows stealing other people's tokens.
Pre-conditions:
withdraw(...)
with some valid signature parameters.The attack:
A malicious user can now call the withdraw(...)
with the same amount
, v
, r
and s
parameters (since they are publicly visible on the block-chain) and transfer the remaining tokens to himself.
The malicious user could call the withdraw(...)
function multiple times, as long as the amount
is still less than the remaining balances[identity]
.
The malicious user could call the withdraw(...)
function again any time later if the balance is topped-up.
Proposed fix:
Introduce an uint256 nonce for each identity:
mapping(address => uint256) public nonces;
Make the nonce part of the signed hash:
function withdraw(uint256 amount, uint256 nonce, uint8 v, bytes32 r, bytes32 s) public returns (bool) {
bytes memory p = abi.encodePacked(WITHDRAW_PREFIX, amount, nonce);
Validate that the identity's nonce has not already been used:
require(nonce > nonces[identity]);
Update the nonce to the latest value:
nonces[identity] = nonce
Important - the above does not help if the withdraw(...)
functions is called with a valid signature, but it reverts due to an insufficient balance. The valid signature would be still publicly visible, but the nonces[identity]
would not be updated. Once the balance is topped-up, this failed transaction could be replayed (until the legitimate user does a new withdraw(...)
transaction, which would update the nonce). There could be a few possible fixes:
also increment nonces[identity]
on every balance modification (in both, topUp(...)
and internalTransfer()
functions). This fix introduces some headaches in the future, since any newly added code which would increase the balance of an identity, should also increment the nonce. Maybe it would be better to encapsulate identity balance increase and nonce modification in a single internal function.
Add an external function to invalide the nonce. Rely on external off-chain code to always call this function in case the withdraw(...)
fails.
Change the code to never revert the withdraw(...)
transaction but update the nonce and return false
if anything goes wrong.
Currently, transactor uses a system where transactions are stored in the database until they succeed or spend a predefined time in the pending state. Once the time is exceeded, the transactions are then retried with a bigger gas price.
This logic is useful for most transactions and should be extracted to the payment package. We could then use the payment package in both hermes and transactor to use the same logic for both.
The logic should abstract away a transaction, to be able to store and retry any transaction independent of the transaction type itself.
Incrementor tests still sometimes race
Currently, we're implementing a sequence ID counter on the provider side. While coding this, an idea popped up.
What happens if we lose the DB, it gets corrupted or the hard drive explodes? In that case we won't be able to know which sequence ID we're currently working with for the given consumer.
So, to restore the sequence ID we need to expose a function on the IdentityPromises contract, to allow to lookup the last cleared sequenceID for a given receiver.
Once that is implemented, the provider can restore the state in case of a disaster. A lookup should not be expensive, so the provider can lookup which seqID it last used for the promise issuer if he does not know anything about the consumer. Query it once, store it in the database and resume normal book keeping on the local DB.
Migration rules:
Whitelist option should be removed from node code with this task.
All tasks in this epic sorted in priority:
Need to have go bindings for the following:
We currently only cache hermes caller: https://github.com/mysteriumnetwork/payments/blob/master/client/hermes_caller.go
I see no reason to stop there, we should cache all callers and transactor for improved CPU usage in services in expense of a minuscule RAM usage increase. Make sure no race conditions are created with this, probably a mutex should be used.
Born from this: #213
This repo is a building block for many other projects so I think we should have proper docs for it. It will also be useful to have a mental model of it and how it works. I can do it so it'll be useful for the new guy + for me to understand it better.
Documentation: https://docs.metamask.io/guide/registering-your-token.html#code-free-example
This should allow user to click link in our documentation and get prompt in Metamask to add MYST.
Go ethereum uses the standart library JSON package which is slow and inefficient. It does a lot of marshaling and unmarshaling. It would be great to do it more efficiently so we can use less memory/cpu.
"github.com/json-iterator/go"
Right now courier is not very modular and clunky to use. It would be nice if we could load it dynamically instead of relying on switch statements.
There seems to be a problem in mainnet where if we give too little gas the transaction takes a while to appear on the blockchain causing us to assume it has failed with an error not found
If current gas price is 30 and we give a low 20 or less even a timer of check every 2 minutes
can still result in not found
and instant fail for a transaction even though it still exists.
The whole workflow with not found errors should be smarter.
A declarative, efficient, and flexible JavaScript library for building user interfaces.
๐ Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. ๐๐๐
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google โค๏ธ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.