Introduce "IssueTokensFromGarage" action #295
Conversation
| @@ -20,5 +23,14 @@ def long_format(self): | |||
| def short_format(self): | |||
| return self.raw.hex() | |||
|
|
|||
| def derive(self, key: str) -> Address: | |||
There was a problem hiding this comment.
주소유도하는 메서드가 어디 따로 있었던걸로 기억하는데, 둘이 결과가 같을까요? 다르다면 고쳐야할것같고, 같다면 하나로 통일하는게 좋을것같습니다.
There was a problem hiding this comment.
네 저도 어디 만들어둔 것 같아서 찾아봤는데 안보여서 만들었습니다. 혹 정리하다 보이면 이쪽으로 모으는걸 생각하고 있습니다
There was a problem hiding this comment.
여기 에 있는데 return type 이 좀 다르네요. derive_address() 를 쓰는 곳은 garage address 유도를 위해 사용하는 곳 하나뿐인데, hash 를 만드는 것은 같고, return 할 때 마지막에 digest 하는 부분이 달라 이건 확인해서 통일해두겠습니다.
There was a problem hiding this comment.
새로 만든 코드로 통일할거라서 기존 코드에는 deprecate 표기 먼저 해놨습니다.
실제로 예전 버전 derive 를 사용하는 곳은 한곳인데, 이건 string > Address 로 타입 변경부터 크게 들어가야 해서 별도 이슈로 만들어두겠습니다.
| @@ -179,8 +179,9 @@ def __init__(self, scope: Construct, construct_id: str, **kwargs) -> None: | |||
|
|
|||
| if config.stage == "mainnet": | |||
| ten_minute_event_rule.add_target(_event_targets.LambdaFunction(status_monitor)) | |||
| else: | |||
| hourly_event_rule.add_target(_event_targets.LambdaFunction(status_monitor)) | |||
| # If you want to test monitor in internal, uncomment following statement | |||
There was a problem hiding this comment.
사실상 필요가 없는데 internal channel 에 noisy 하고 mainnet 과 헷갈리게 메세지가 와서 테스트가 필요한 기간이 아니면 끄면 좋겠다는 의견을 받았습니다
- Deprecate old function
- Only one usage: need to be changed
- Change new function
- Use encode_checksum logic instead of hexdigest
- keys are not set - Update return values
| @@ -24,6 +24,8 @@ on: | |||
| required: true | |||
| KMS_KEY_ID: | |||
| required: true | |||
| ADHOC_KMS_KEY_ID: | |||
There was a problem hiding this comment.
토큰 발행용 계정인걸까요? �ADHOC보단 별도의 정해진 이름이 있으면 좋을것같네요.(지금 당장 고쳐야할 건은 아닙니다.)
There was a problem hiding this comment.
네 garage > issue token 하는 계정입니다.
처음에 만든건 예전 IAP 초기에 수동지급 필요할 때 만들어서 adhoc 으로 한건데, 이름이야 뭐 고치면 됩니다 ㅋ
| @@ -21,10 +21,13 @@ def fetch_secrets(region: str, secret_arn: str) -> Dict: | |||
| return json.loads(resp["SecretString"]) | |||
|
|
|||
|
|
|||
| def fetch_kms_key_id(stage: str, region: str) -> Optional[str]: | |||
| def fetch_kms_key_id(stage: str, region: str, adhoc: bool = False) -> Optional[str]: | |||
There was a problem hiding this comment.
코드를 봐서는 서명용 키를 워커마다 다르게 가져가는 식인것같은데, 중복이 좀 되더라도 차라리 키별로 얻어오는 메서드를 아예 명시적으로 나눠버리는게 어떨까 싶네요.
There was a problem hiding this comment.
이걸 True 로 쓰는 데가 사실상 manual sign 하는 곳 (운영 계정에서 IssueToken + Transfer Assets) 밖에 없기는 한데
근데 저걸 또 가끔은 IAP 판매 계정으로 써야 할 때가 있어서요.
완전히 worker 별로 얘는 이거 쟤는 저거로 고정되어 있지는 않습니다.
There was a problem hiding this comment.
A계정으로 B계정의 일을 처리하는게(혹은 반대) 겁나는 상황인건데, 이걸 막을수있을까요? (tx에 들어간 액션따라 허용 주소를 둔다던지)
There was a problem hiding this comment.
IssueTokensFromGarage action 의 경우 무조건 context.Signer 주소를 가지고 작업을 하도록 되어 있어서 다른 계정의 재고를 건드리는 일은 없습니다.
그리고 우려하시는 A 계정의 garage > token 작업을 해야 할 때 B 계정으로 서명을 하는 경우에 대해서는
- 어차피 이제 모든 garage 는 token 으로 갈 거니 넘어가도 큰 문제는 안 된다
- 주소를 잘못 넣으면 garage 가 비어 있어서 에러가 날거다
정도가 발생할 것으로 보여서 크게 걱정은 안했습니다.
Derivefunction to get derived addressIssueTokensFromGarageaction