Skip to content

Conversation

@riptl
Copy link
Contributor

@riptl riptl commented Jan 6, 2020

Description

Adds support for Filecoin, implementing a secp256k1-based wallet.
(Old PR #784)

Changes

  • Filecoin support (with secp256k1 wallet)

Checklist

Closes #780.

@codecov
Copy link

codecov bot commented Jan 6, 2020

Codecov Report

Merging #811 into master will increase coverage by 0.06%.
The diff coverage is 94.05%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #811      +/-   ##
==========================================
+ Coverage   90.09%   90.16%   +0.06%     
==========================================
  Files         345      351       +6     
  Lines       11272    11457     +185     
==========================================
+ Hits        10156    10330     +174     
- Misses       1116     1127      +11
Impacted Files Coverage Δ
src/Coin.cpp 94% <0%> (-1.92%) ⬇️
src/Filecoin/Signer.cpp 100% <100%> (ø)
src/Filecoin/Transaction.cpp 100% <100%> (ø)
src/Filecoin/Transaction.h 100% <100%> (ø)
src/interface/TWFilecoinSigner.cpp 100% <100%> (ø)
src/Filecoin/Address.h 89.47% <89.47%> (ø)
src/Filecoin/Address.cpp 92.78% <92.78%> (ø)
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ef107a1...25408e2. Read the comment docs.

@riptl riptl force-pushed the filecoin branch 3 times, most recently from 16af938 to 6e3fa34 Compare January 7, 2020 17:15
@riptl
Copy link
Contributor Author

riptl commented Jan 18, 2020

Transaction generated by wallet-core now considered valid by Lotus node: 828855018ac93840e869c06b79b748a3c504b696bb339a7f5501cf01bf485f61435e6770b52615bf455e043a236101430017704200024200c80040584201477bcf9a71c9e19af77fbd92677230a9612d927877e439eb4aa306430a4ffae42251194820df211d00075e88e68c9704a880448532ec0092045a1e00e8f1cbb900

Sample request to push transaction as base64 to node (requires filecoin-project/lotus#1108)

{
	"jsonrpc":"2.0",
	"id":1,
	"method":"Filecoin.MpoolPush",
	"params":[
		"gohVAYrJOEDoacBrebdIo8UEtpa7M5p/VQHPAb9IX2FDXmdwtSYVv0VeBDojYQFDABdwQgACQgDIAEBYQgFHe8+accnhmvd/vZJncjCpYS2SeHfkOetKowZDCk/65CJRGUgg3yEdAAdeiOaMlwSogESFMuwAkgRaHgDo8cu5AA=="
	]
}

Gas spent was 126. The node uses 1000 as the default value for gas_limit.
Nonce needs to start at 1 and increments from there.

Message correctly deserialized by Lotus node:

(*types.SignedMessage)(0xc01e003830)({
 Message: (types.Message) {
  To: (address.Address) t1rletqqhinhagw6nxjcr4kbfws25thgt7owzuruy,
  From: (address.Address) t1z4a36sc7mfbv4z3qwutblp2flycdui3baffytbq,
  Nonce: (uint64) 1,
  Value: (types.BigInt) 6000,
  GasPrice: (types.BigInt) 2,
  GasLimit: (types.BigInt) 200,
  Method: (uint64) 0,
  Params: ([]uint8) {
  }
 },
 Signature: (types.Signature) {
  Type: (string) (len=9) "secp256k1",
  Data: ([]uint8) (len=65 cap=65) {
   00000000  47 7b cf 9a 71 c9 e1 9a  f7 7f bd 92 67 72 30 a9  |G{..q.......gr0.|
   00000010  61 2d 92 78 77 e4 39 eb  4a a3 06 43 0a 4f fa e4  |a-.xw.9.J..C.O..|
   00000020  22 51 19 48 20 df 21 1d  00 07 5e 88 e6 8c 97 04  |"Q.H .!...^.....|
   00000030  a8 80 44 85 32 ec 00 92  04 5a 1e 00 e8 f1 cb b9  |..D.2....Z......|
   00000040  00                                                |.|
  }
 }
})

@riptl
Copy link
Contributor Author

riptl commented Jan 18, 2020

@hewigovens I will add back BigInt support uint256_t for values. Lotus is using 18 decimal places, not 8!

@hewigovens
Copy link
Contributor

@terorie Lmao, totally different order of magnitude

@riptl riptl force-pushed the filecoin branch 2 times, most recently from d3b409e to b03a970 Compare January 19, 2020 12:59
@optout21
Copy link
Contributor

@hewigovens I will add back BigInt support uint256_t for values. Lotus is using 18 decimal places, not 8!

Don't forget to update that in coins.json as well.

@optout21
Copy link
Contributor

optout21 commented Jan 20, 2020

@hewigovens , should this be done already using AnyAddress now?

@hewigovens
Copy link
Contributor

@terorie can you rebase master? we don't need to have a new C interface for filecoin address, we will adopt TWAnyAddress as much as possible

@riptl riptl force-pushed the filecoin branch 3 times, most recently from 51b8666 to 0c8a23f Compare February 10, 2020 09:17
@riptl
Copy link
Contributor Author

riptl commented Feb 10, 2020

@riptl
Copy link
Contributor Author

riptl commented Feb 10, 2020

@vikmeup @hewigovens Transactions generated by wallet-core were successfully mined on testnet! CID of mined SignedMessage: bafy2bzacebrz2m4otsmkxbx4kzdu2csx35dz3p3ap7tys3pnol6oyl5s3ts4s

It's ready for review+merge from my side.

Copy link
Contributor

@hewigovens hewigovens left a comment

Choose a reason for hiding this comment

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

nice work, left some comments (and there is one codacy warning)

Copy link
Contributor

@optout21 optout21 left a comment

Choose a reason for hiding this comment

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

Nice work, thanks for all the reworks!

@riptl riptl force-pushed the filecoin branch 4 times, most recently from 9ed8598 to de5fdd2 Compare February 13, 2020 22:08
@riptl
Copy link
Contributor Author

riptl commented Feb 13, 2020

@catenocrypt Good remarks, I fixed the remaining issues, squashed & rebased onto master!
I also got Codacy happy, so all tests now pass.

@hewigovens hewigovens requested a review from optout21 February 13, 2020 22:41
@optout21 optout21 merged commit b0baaf3 into trustwallet:master Feb 13, 2020
@riptl riptl deleted the filecoin branch February 14, 2020 10:56
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.

Add support for Filecoin

4 participants