by

Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Repo info
Activity
  • Jul 29 01:16
  • Jul 22 07:07
  • Jul 22 07:07
    Travis kseo/codechain-rpc-js (license) passed (1)
  • Mar 20 06:28
    MSNTCS opened #1962
  • Mar 20 06:27
    MSNTCS commented #1961
  • Mar 20 06:25
    MSNTCS closed #1961
  • Mar 19 08:02
    MSNTCS review_requested #1961
  • Mar 19 08:02
    MSNTCS opened #1961
  • Mar 03 02:37
    ScarletBlue edited #1948
  • Mar 03 02:37
    ScarletBlue edited #1931
  • Mar 02 00:41
    ScarletBlue edited #1915
  • Mar 02 00:41
    ScarletBlue edited #1915
  • Feb 27 00:09
    sgkim126 closed #1959
  • Feb 27 00:09

    sgkim126 on master

    Bug fix in on_request_commit_me… (compare)

  • Feb 25 09:41
    majecty labeled #1960
  • Feb 25 09:41
    majecty opened #1960
  • Feb 17 02:10
    majecty commented #1959
  • Feb 14 23:13
    sgkim126 review_requested #1959
  • Feb 14 23:13
    sgkim126 review_requested #1959
  • Feb 14 23:13
    sgkim126 opened #1959
Joonmo Yang
@Remagpie
Thanks. I almost forgot that. I added some commits to the PRs I sent.
Seulgi Kim
@sgkim126
@majecty I think we'd better implement shard module for testing including a dummy transaction.
Park Juhyung
@majecty
OK. I'm adding a simple transaction that uses shards. I'll request your Review today. Then I'll remove other shard transactions.
Park Juhyung
@majecty

Our check_shard_level_state macro is an excellent point to study Rust macro. I understood how to use the macro by reading it.

https://github.com/CodeChain-io/foundry/blob/master/state/src/impls/test_helper.rs#L558

Joonmo Yang
@Remagpie
Are we maintaining simple_poa consensus in foundry? I can't remember the usage of it.
Seulgi Kim
@sgkim126
The purpose was a testing network consensus without tendermint logic, but we don't use it for now.
Seulgi Kim
@sgkim126
I'll do CodeChain-io/foundry#21, CodeChain-io/foundry#14 would be better doing after snapshot refactoring.
Seulgi Kim
@sgkim126
Hi @majecty and @kseo, While I'm reviewing and writing commits, I found that the current coding style about importing order is easy to make a mistake. https://github.com/CodeChain-io/codechain/wiki/Coding-Style Currently, we split imports into 3 groups that are systems, other packages, and self modules, and sorting them in a group, but there are no formatting tools that group imports according to prefixes.
So we split the groups with a blank line, but we should take care of whether they are grouped as intended; it's easy to make a mistake.
I suggest to remove groups and make all imports ordered alphabetically. AFAIK, rust compilation doesn't affect by the importing order; therefore, it will not make a problem.
Kwang Yul Seo
@kseo
@sgkim126 sounds good to me. @majecty what’s your opinion?
Park Juhyung
@majecty
I'm happy to hear the suggestion. Let's do it!
Kwang Yul Seo
@kseo
@sgkim126 please file an issue first
Seulgi Kim
@sgkim126
JinGyeong Jeong
@joojis
I think there is a point of pain when a new contributor is trying to patch RPC because of the e2e tests.
I think we should change our practice to use sendRpcRequest function in our e2e tests so that no SDK publish is required.
Kwang Yul Seo
@kseo
@sgkim126 would you review this PR again?
CodeChain-io/foundry#12
Seulgi Kim
@sgkim126
Park Juhyung
@majecty
FYI, it's 2020! Let's update copyright when we modify files.
Park Juhyung
@majecty
@sgkim126 I updated the CodeChain-io/foundry#12 PR.
Since the PR is created in the remagpie's branch, I cannot update PR.
Should I close the PR and re-create it?
Park Juhyung
@majecty
^ I sent a new PR CodeChain-io/foundry#120
It is funny that the new PR's number = 120 = 10 * the previous PR's number.
I'll import CodeChain's ICS implementation PR to Foundry.
CodeChain-io/codechain#1849
Park Juhyung
@majecty
^ Done.
The previous ics-poc was using the custom action handler. Since Seulgi will remove custom action soon, I'll migrate custom actions to normal transaction types.
Seongjae Choi
@cubismic
What is regular_key_owner? Is it just master_account that has the regular_key?
Park Juhyung
@majecty
@cubismic Yes, you're right. It is the master account.
Seongjae Choi
@cubismic
Thank you!
Park Juhyung
@majecty
I'm fixing this PR: CodeChain-io/foundry#120
Park Juhyung
@majecty
DONE.
Park Juhyung
@majecty
I'm reviewing CodeChain-io/foundry#116
Park Juhyung
@majecty
@sgkim126 I reviewed your PR: CodeChain-io/foundry#116
Park Juhyung
@majecty
DONE
Park Juhyung
@majecty
I completed "Migrate IBC transactions to use TopLevel transaction" PR.
I'm checking ICS connection spec to check which functions I should implement.
https://github.com/cosmos/ics/tree/master/spec/ics-003-connection-semantics
Park Juhyung
@majecty
I misunderstood the role of the relayer in IBC.
I thought the relayer don't need to know the exact format of datagrams.
I found that the relayer should create diagrams by querying the state of chains.
Park Juhyung
@majecty
I've started implementing connection in IBC.
Park Juhyung
@majecty
While writing code of connection, I found that our ics-poc CommitmentPrefix is too generic. Maybe I'll fix it soon.
junha1
@junha1
I think most of ICS code should be concrete in ics-poc branch, since they're designed solely for Foundry as an opposite chain. Instead, interfaces that will be embedded in module should be generic.
Park Juhyung
@majecty
Yes, let's talk about how to change current ics-poc branch not to generic today.
In the connection spec, they are saying, "Each chain has knowledge of and has agreed to its identifier on the other chain."
We might need some authorization logic to make only allowed users be able to create connections.
junha1
@junha1
Then how about just having that (set of identifiers) in state since the Genesis?
Byeongjee Kang
@Byeongjee
I'm implementing BLS signature aggregation scheme in foundry. Introducing new hash types such as H384 and H768 will be really helpful. Is there any nice way to do this?
Currently I forked rust-codechain-primitives repository and added the new types with fixed_hash library. However I had to copy impl_serde macro from ethereum-types crate. There is impl_serde crate, but I couldn't make use of it because ethereum-types library uses a very old version of fixed_hash library, so they were not compatible.
I also had to fork and add code in codechain rlp to give the new types Encodable/Decodable traits.
Park Juhyung
@majecty

Then how about just having that (set of identifiers) in state since the Genesis?

Maybe that can be a solution. I'll create a GitHub issue about this.

Byeongjee Kang
@Byeongjee

Anyone know about history of parameter names in execute method of ActionHandler trait?

I think the name 'fee_payer' makes it hard to understand. The address can be read as fee payer for money transfer, but I think it should be 'deposit_owner' in self nomination and 'informant' in double vote reporting.

Does the fee_payer actually pay fee? If not, I will make a PR to change its name to more understandable one, such as 'sender_address'.

Park Juhyung
@majecty
Here is the history. In CodeChain, anyone can create assets and give them to anyone. When transferring an asset, the asset owner and a fee payer should sign the transaction. So 'send_address' is a confusing name.
In Foundry, since we don't have a native asset system (anyone can create asset using module), we can use the "sender_address"
junha1
@junha1
How about getting ibc directory out of core? And we can make it as a separate crate. @majecty
Park Juhyung
@majecty
@junha1 I agree with you.
It seems we don't need to hurry. How about moving it after PoC implementation?
junha1
@junha1
No problem