Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

🆙 Chore: packages & node update #43

Open
wants to merge 42 commits into
base: main
Choose a base branch
from
Open

🆙 Chore: packages & node update #43

wants to merge 42 commits into from

Conversation

sk-enya
Copy link

@sk-enya sk-enya commented Oct 14, 2024 •
edited
Loading

Closes: #41

Change

  • Node version to 20+
  • use of pnpm instead of yarn
  • updated package scripts as separate for test:unit test:e2e
  • Updated docker file to build and test setup service with pnpm
  • lb-tests ci update to docker compose
  • docker compose change expose port for anvil with platform force to amd64
  • cleanup light bridge service from based as it's been prepared in reg and parellel
  • update lb script to use docker compose

@meanMonk
changes
0129354 
- node version to 20+
- moved yarn to pnpm
- updated docker file to build and test setup service with pnpm
- lb-tests ci update to `docker compose`
- docker compose change expose port for anvil with platform force to amd64
- cleanup light bridge service from based as it's been prepared in reg and parellel
@meanMonk
linting and formatting change!
fe033a7
@sk-enya sk-enya requested review from wsdt and boyuan-chen October 14, 2024 04:59
@sk-enya sk-enya mentioned this pull request Oct 14, 2024
Closed
@meanMonk
Exit fee updated contract :
f2f1c11 
- Added mapping variable for percentExitFee `{sourceChainId} => {percentExitFee}`
- Added variable to record the collectedFee.
- Event to capture the PercentExitFee or change
- Function to calculate the amountAfterFee deduction with input as amount and sourceChainId.
- update disburseAsset fn to use the calculateAmountAfterFee and fee deducted to keep record.
- function to set PercentExitFee corresponding to sourceChainId only owner can set it.
@meanMonk
spce for set percentExitFee and event tracking of percentExitFeeSet
ef64747
@meanMonk
change fn name fixes
1c8ac02
@meanMonk
unit test cases for calculateAmountAfterFee
cd817ed
@meanMonk
linting change
395f852
@meanMonk
added unit test to validate fee deduction logic
587c64f
@meanMonk
cleanup deduct exit fee logic from service
ee46b63
@meanMonk
format fixes
dda6cc6
@wsdt
Copy link
Contributor

wsdt commented Oct 14, 2024

image

Nice! Seems like there is just a public node not working rn.

Do the tests run locally for you if you define the RPC environment variables with some Tenderly private nodes? @sk-enya

@sk-enya
Copy link
Author

sk-enya commented Oct 14, 2024

Can you please help me with tenderly L2 url, I am unable to find that! for sure it will start working as soon as our L2 sepolia will be back.

@wsdt
Copy link
Contributor

wsdt commented Oct 14, 2024

Can you please help me with tenderly L2 url, I am unable to find that! for sure it will start working as soon as our L2 sepolia will be back.

if Boba Sepolia is down @boyuan-chen or @jyoti-enya might be better to help with that as you suggested in our chat. :-)

@meanMonk
change: updated test content
5d04dda
@meanMonk
change: updated test label
f254219
@meanMonk
Merge branch 'feat/lb-update-exit-fee-with-record-collection' of gith…
9284009 
…ub.com-enya:bobanetwork/light-bridge into feat/lb-update-exit-fee-with-record-collection
@meanMonk
change:
a0fe217 
- feeCollectRecord mapping(address => uint256)
- updated feeCollectRecord
- updated unit test cases for checking values in feeCollectRecord
@meanMonk
change name to destinationChainId & update 10000 to 10_000 for better…
3192d5c 
… naming
@sk-enya sk-enya mentioned this pull request Oct 21, 2024
Open
boyuan-chen and others added 19 commits November 1, 2024 10:29
@boyuan-chen
Use contract read instead of subgraph
81c60b3
@boyuan-chen
Remove Tenderly URL
20a3f37
@boyuan-chen
Use contract event
969704b
@boyuan-chen
Use contract events
d4417e9
@boyuan-chen
Add a random transaction
7d8b2b3
@boyuan-chen
Fix event
1ac88f9
@boyuan-chen
Add a random tx
ecb27db
@boyuan-chen
Fix contract filter
de4f927
@boyuan-chen
Get tx correctly
2c3d516
@boyuan-chen
Fix parallel test
f9c4a0e
@boyuan-chen
Fix parallel test
c158b6d
@meanMonk
change variable name to sourceChainId + enable skip test
7664bb8
@meanMonk
fix broken test
814c42b
@meanMonk
lint fixes
745156d
@meanMonk
reverted the test fix and updated startup test
c7cc9a1
@meanMonk
formating
f266364
@meanMonk
fix: broken test and enable other spec to run
d4c40d7
@sk-enya
Merge pull request #44 from bobanetwork/feat/lb-update-exit-fee-with-…
2e597de 
…record-collection

Feat: ExitFee in percentage logic move to contract.
@meanMonk
Merge branch 'main' of github.com-enya:bobanetwork/light-bridge into …
d1ff664 
…chore/packages-node-update
@boyuan-chen boyuan-chen mentioned this pull request Nov 13, 2024
Open
@wsdt
Copy link
Contributor

wsdt commented Nov 13, 2024

Before we merge this:

  • @sk-enya @boyuan-chen want to upgrade the LightBridge contracts on testnet and update the testnet backend?

Huge change.

And also, do we plan to re-audit the contract? Or what's the thought process here.

@boyuan-chen
Copy link
Contributor

Before we merge this:

  • @sk-enya @boyuan-chen want to upgrade the LightBridge contracts on testnet and update the testnet backend?

Huge change.

And also, do we plan to re-audit the contract? Or what's the thought process here.

Yes, please.

I don't think we need another round of the audit. The changes are very minor. The core logic of sending funds doesn't change.

wsdt
wsdt reviewed Nov 13, 2024
View reviewed changes
test/lightbridge.integration.spec.ts
blockNumber
)
const events =
await teleportationService._getAssetReceivedEventsViaQueryFilter(
Copy link
Contributor

@wsdt wsdt Nov 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

ah that's why the tests might be going through here. We just don't test the graphutils package in combination with the lightbridge anymore.

Could you check if tests still pass if replaced with the subgraph as this is what's being used in prod?

Copy link
Contributor

Choose a reason for hiding this comment

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

we should get this fixed as tests are failing through recent graph-utils changes. @sk-enya

wsdt
wsdt requested changes Nov 13, 2024
View reviewed changes
Copy link
Contributor

@wsdt wsdt left a comment

Choose a reason for hiding this comment

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

left a comment and would be great to test this on testnet first.

@sk-enya
Copy link
Author

sk-enya commented Nov 17, 2024

Sure, will review and update comments. yeah make sense to first validate on testnet!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wsdt wsdt wsdt requested changes

@boyuan-chen boyuan-chen Awaiting requested review from boyuan-chen

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add fee logic to smart contract
4 participants
@sk-enya @wsdt @boyuan-chen @meanMonk